On Fri, Feb 16, 2001 at 11:02:05AM -0500, Jim Blandy wrote:
>
> Greg Stein <gstein@lyra.org> writes:
> > On Thu, Feb 15, 2001 at 11:17:58PM -0000, jimb@tigris.org wrote:
> > >...
> > > --- tree.c 2001/02/15 18:58:53 1.16
> > > +++ tree.c 2001/02/15 23:17:58 1.17
> > >...
> > > +static svn_error_t *
> > > +mutable_root_node (dag_node_t **node_p,
> > > + svn_fs_root_t *root,
> > > + const char *error_path,
> > > + trail_t *trail)
> > > +{
> > > + /* If it's a revision root, we can't change its contents. */
> > > + if (root->rev != -1)
> > > + return svn_fs__err_not_mutable (root->fs, root->rev, error_path);
> >
> > What's that magic number there? SVN_INVALID_REVNUM? Something else?
> > Can you use a symbol rather than a free-floating number?
>
> Well, there's only one distinguished value for the `rev' field of an
> svn_fs_root_t. It's documented in the explanation of that field. -1
> is a common quiescent value for quantities for which zero is
> meaningful --- like revision numbers. And there's a comment directly
> above that `if', explaining what it's checking for. So I think the
> code is pretty legible as it stands.
Because of the simple fact that I asked, empirical evidence would say that
the code is /not/ legible.
The comment regarding -1 is a long ways away from the usage, so it is easy
for a person to not know about it (e.g. my case). Further, how would a
person know that -1 represents "uninitialized" or "quiescent" rather than a
meaningful value? I certainly didn't understand that the -1 meant "no
meaning here, just move along."
Bare numbers are always a bit of a problem because the reader can never
truly understand whether they are significant and/or their semantics.
Cheers,
-g
p.s. yes, I know this concrete case doesn't apply; I'm dicussing style now :-)
--
Greg Stein, http://www.lyra.org/
Received on Sat Oct 21 14:36:22 2006