[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] [DOCFIX] Fix ambiguous comments for svn_string_isempty()

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2006-03-10 15:36:05 CET

Madan U S wrote:
> > From: Malcolm Rowe [mailto:malcolm-svn-dev@farside.org.uk]
> > On Fri, Mar 10, 2006 at 11:01:46AM +0530, Madan U S wrote:
> > > When I saw it first, and the next function comment said 'return @c
> TRUE' I was beginning to get confused.
> >
> > Perhaps it would make more sense to switch all the existing 'Return @c
> > TRUE' doc-comments to 'Return true ...' instead. If it bothers you,
> > anyway.
> no, the next function actually returns the svn defined bool value
> 'TRUE', not just an integer.

The current implementations of both functions return the integer 1 (of type
svn_boolean_t). "TRUE" is not the kind of macro whose value might ever change,
so there is no difference between the two functions at the API level.

This is how I think of the rules for boolean values in C:

When reading a boolean value,

* zero is false (as established by the built-in boolean operators)

* non-zero is true (as established by the built-in boolean operators)

Good code should follow this principle and not rely on a true value being
exactly 1 even if the author knows that the value came from an operation that
guarantees to output exactly 1 for truth.

When writing a boolean value,

* false is (int) 0 (as established by the built-in boolean operators)

* true is (int) 1 if a particular value is chosen (as established by the
built-in boolean operators, which always return 1 for "true")

* true is any non-zero int otherwise (as established by standard
functions/macros like isupper(), which typically return the result of a
bit-wise AND)

We provide TRUE and FALSE macros as self-documenting synonyms for 1 and 0, and
these should be used where an explicit boolean value is needed.

It follows from these rules that I consider any test for (b == TRUE) or (b !=
TRUE) to be wrong because the value might represent "true" but not be
numerically equal to the macro "TRUE", depending on where it came from. I find
expressions like (b == FALSE) distasteful by association.

In API functions such as the two that were initially commented on,
svn_string_isempty() and svn_string_compare(), the caller should not need to
know nor care what value is returned, only that it is non-zero. In my opinion,
a different implementation of the same API should be free to return a different
value. The doc-string of an API is primarily meant for the caller, and it
should not encourage the caller to depend upon a particular non-zero value
being returned.

We should change "@c TRUE" to "true" in our API documentation, for both inputs
and outputs, both because it avoids assumptions about what exact value is used
and whether it should be relied upon, and because it is neater and less
distracting to read. May I?

It would also be good if we documented somewhere (probably at the definition of
svn_boolean_t) that "true" has the meanings I described above. Perhaps:

-/** YABT: Yet Another Boolean Type */
+/** Our own Boolean type. To provide a value, use TRUE or FALSE if an
+ * explicit value is needed, else the non-zero or zero result of an
+ * expression. Use the value in an implicit test for truth such as
+ * "(... && b)" or "if (! b)", not a comparison such as "(b == TRUE)". */
  typedef int svn_boolean_t;

- Julian

To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Mar 10 15:40:12 2006

This is an archived mail posted to the Subversion Dev mailing list.