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

RE: svn commit: r25554 - trunk/subversion/libsvn_wc

From: Paul Burba <pburba_at_collab.net>
Date: 2007-06-27 17:15:34 CEST

> -----Original Message-----
> From: Malcolm Rowe [mailto:malcolm-svn-dev@farside.org.uk]
> Sent: Wednesday, June 27, 2007 10:39 AM
> To: dev@subversion.tigris.org; pburba@tigris.org
> Subject: Re: svn commit: r25554 - trunk/subversion/libsvn_wc
>
> On Wed, Jun 27, 2007 at 06:55:20AM -0700, pburba@tigris.org wrote:
> > Set TRUE/FALSE *not* non-zero/zero int when testing a WC
> path's switch status.
> >
> > * subversion/libsvn_wc/util.c
> > (svn_wc__path_switched): Set *switched argument to TRUE
> or FALSE as per the
> > docstring for this function.
>
> In general, our doc-strings should just say something like
> "Set *switched to true if ...; otherwise, set *switched to
> false", and not worry about whether we're using TRUE specifically.

Hi Malcolm,

In the past I've gotten advice in different directions on this question.
Certainly we are inconsistent in our docstrings regarding "svn_boolen_t
*pred" type arguments.

We commonly use

  "set *pred to true"
  "set *pred to TRUE"
  "set *pred to @c TRUE"

But we also spice things up with:

  "set @a *changed_p to 1..."
  "Set @a *modified_p to non-zero"
  "return the answers in @a text_conflicted_p and @a prop_conflicted_p."
  
And %DEITY% knows what else...this was just a quick look on my part.

Implementations are also varied, some setting TRUE/FALSE explicitly some
using the result of a conditional statement.

I am not married to any of these, whatver folks agree is the best way is
what I'll use.
 
> (and we obviously shouldn't be testing again TRUE directly).
>
> > - *switched = strcmp(parent_child_url, entry->url);
> > + *switched = strcmp(parent_child_url, entry->url) == 0 ? FALSE :
> > + TRUE;
>
> (Incidentally, I find both of those forms really hard to
> parse. That might just be me, however).

Heh, ok, I admit it, I'm in love with the ternary operator. But if
there is anything even approaching general agreement that the above
examples hinder code readability, I'm more than happy to use something
like this from now on:

if (strcmp(parent_child_url, entry->url))
  *switched = TRUE;
else
  *switched = FALSE;

Hopefully this is not a bikedshed issue!

Thanks,

Paul

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jun 27 17:17:00 2007

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.