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

Re: [PATCH-REVIEW] Relative URL support for info command

From: Stefan Sperling <stsp_at_elego.de>
Date: 2007-10-27 19:07:45 CEST

On Sat, Oct 27, 2007 at 10:57:46AM -0500, Troy Curtis Jr wrote:
> **NOTE: I am asking for a quick review of my code.

I had a quick look over it.

Found nothing bad, except this piece:

        svn_path_is_relative_url(const char *url)
            if(0 == strncmp("^/", url, 2))
                return TRUE;
                return FALSE;

First, you are missing a space after 'if'.
See http://subversion.tigris.org/hacking.html#coding-style
It helps if all code adheres to the conventions.

Second, you could simply do this instead:

        svn_path_is_relative_url(const char *url)
            return (0 == strncmp("^/", url, 2));

"If (true) return true; else return false;" is redundant.

But (as I assume) in case anyone ever swaps the values of TRUE and
FALSE in svn_types.h (oh, the horror), I've seen this idiom being
used instead in the subversion code:

        svn_path_is_relative_url(const char *url)
            return (0 == strncmp("^/", url, 2) ? TRUE : FALSE);

I think this looks ridiculous as well but it's being used
in many places, and it fits on one line. There is probably
some good reason for it... (Is there?)

> My function prototypes are not properly documented yet

My advice: Don't do this! Instead:

        Document your functions *before* writing them.

Then keep the docs in sync with the code whenever you change

This helps you keep thinking about what you're doing, and you
actually have the docs ready by the time you want to send a patch.

The docs also help others because they explain your intentions.
Other people can use this information to help you verify that
your intentions match what the code actually does.
And no, a quick summary of what a function should do in the
body of your submission email does not count as documentation.

The amount and quality of the documentation inside the Subversion
code base is one of its best advantages. Don't miss out on it.

Lack of docs makes your code less accessible.
"Code without docs is like hardware without specs." :)

Stefan Sperling <stsp@elego.de>                 Software Developer
elego Software Solutions GmbH                            HRB 77719
Gustav-Meyer-Allee 25, Gebaeude 12        Tel:  +49 30 23 45 86 96 
13355 Berlin                              Fax:  +49 30 23 45 86 95
http://www.elego.de                 Geschaeftsfuehrer: Olaf Wagner

  • application/pgp-signature attachment: stored
Received on Sat Oct 27 19:08:04 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.