New wiki entry on code quality considerations

Well, I must admit that this post from Amenel’s blog made me think a bit about the need to have a clearly written policy on code quality standards as soon as you begin to have contributors. And even when you’re alone like me, in fact, to make sure that this policy doesn’t gradually changes in your mind as time passes.

So here it is : the entry on code quality on the project’s wiki. Please check it out and report anything which looks suspicious, it’s best to apply modifications to such rules before potential contributors have already read them ;)

3 thoughts on “New wiki entry on code quality considerations

  1. Amenel November 28, 2010 / 12:32 am

    A few comments regarding the wiki entry:

    – 2-character indent size seems oddly low, especially when tabs are forbidden (and can’t be revealed by displaying hidden whitespace characters). Because 1- 4-character seems to be the standard (I mean with Java as I am no longer on the C/C++ scene) and 2- I can’t wrap my brain around anyone still editing code on a 1024×768 screen. So 2-character indent would foster deeply nested braces, which I hate (almost as much as Vista) . Endless nesting is an obvious call for subroutines. I for one use 1440×900 screens either at home or at work and I make Eclipse wrap lines at 100-column width using hard breaks. I am still annoyed though with having to use the horizontal scrollbar and the mouse sometimes. I can’t wait to build my own killer desktop machine with a full HD screen.

    – you might want to expand LOC as “line of code”. It’s not obvious (hint hint!) at first reading.

    – “Use comments anytime what you’re doing is not perfectly obvious to an exterior person”. No no no. I have been a university teacher and I can tell you that human communication should never include the dangerous pitfall that “obvious” is. I make meaningful Javadoc mandatory. Anything should be documented, either at the field/variable level or at the “general description” level. The code snippet below is from one of my utility classes. The containing class is called DomUtils and is intended for searching and manipulating XML files loaded using a DOM parser. From the method description, it’s “obvious” that a list of nodes will be returned. This is Java, the list may be empty or null when there’s no child. If I hadn’t written the semantics down, I would have implicitly left room for guessing. I think guessing is bad in anything inherently reusable like code. Imagine if chips were manufactured without marks…

    /**
     * Gets all immediate children as a list.
     * 
     * @param rootElt
     *            the root element
     * 
     * @return the children list. Never null.
     * @since 1.0.0
     */
    public static List getChildren(Element rootElt) {
    ...
    

    A parameter is named “rootElt” but I still write down that it’s the root element… twenty seconds to write, an eternity of unambiguousness. who knows what notion “Elt” may convey to a Finnish or Australian person? I can’t guarantee they’ll read it as “Element”; maybe that in their culture “Element” is shortened as “Elem”? I’m not sure I’m being clearer but my point is that there should be only one interpretation of code, just like CPUs do. In other words, if I need to read the code to retrieve preconditions, precautions, etc. then it’s been (purposely) badly written. Yes, I’m an extremist but beans I write only have a class-level documentation.

  2. Hadrien November 28, 2010 / 1:59 pm

    – 2-character indent size seems oddly low, especially when tabs are forbidden (and can’t be revealed by displaying hidden whitespace characters). Because 1- 4-character seems to be the standard (I mean with Java as I am no longer on the C/C++ scene)

    Well, it’s obviously highly subjective. As someone who has always used 2-char indents (it was the default on my first programming tool, Delphi 5), I’d say that it’s 4-character indents which seem oddly big and look like flying wasted space.

    In the C/C++ world, there’s no established standard in that area : bochs uses 2-char indent, GCC uses 2-char indents too (or 3-char sometimes, it’s weird), kde uses 4-char indent, linux uses 8-char indent… But if it’s the norm in the Java world, I should probably switch to that and spend some times rewriting existing code, though. For some reason, every single newly formed professional programmer has to learn Java at some point, so they’ll probably all be more familiar with 4 spaces indent…

    and 2- I can’t wrap my brain around anyone still editing code on a 1024×768 screen.
    (…)
    I for one use 1440×900 screens either at home or at work and I make Eclipse wrap lines at 100-column width using hard breaks. I am still annoyed though with having to use the horizontal scrollbar and the mouse sometimes. I can’t wait to build my own killer desktop machine with a full HD screen.

    Please note that actually, your problem is less about pixel resolution than about font settings and screen area. As an example, 12pt Times New Roman have caps ~4mm tall and minuscules ~2mm tall, regardless of whether you are in 800×600 or full HD, as long as your screen is set up properly in the operating system’s settings.

    A high pixel resolution is useful when what you want is precision, not screen space. That’s why 30″ HDTVs are almost perfectly useless : from the distance you’re watching a TV, you’ll have a hard time seeing the extra details compared with SD TVs of the same size. In the same way as the iPhone 4’s razor-sharp display is looking better and is probably easier to read on, but that it doesn’t magically make a 3.5″ screen suitable for extensive mailing or word processing.

    To sum it up : if you want to see more text at once, shrink font size or buy a bigger screen, but HD alone won’t do the trick ;)

    So 2-character indent would foster deeply nested braces, which I hate (almost as much as Vista) . Endless nesting is an obvious call for subroutines.

    That, on the other hand, is a very, very good other argument for 4-char indent. Okay, I’m sold ;)

    you might want to expand LOC as “line of code”. It’s not obvious (hint hint!) at first reading.

    WTF ??? Dammit, I thought I had avoided those pesky abbreviations successfully at first look, but there was a nasty one remaining around… *squick* Problem solved

    More coming later…

  3. Hadrien November 28, 2010 / 10:40 pm

    About the “obvious” part… I just knew that it was wrong when writing it, but I don’t know how to express what I was thinking better.

    Regularly, when reading headers from popular open-source projects, I find comments which are best described as “utterly useless”. Take this masterpiece from elf.h as a good example :
    unsigned char e_ident[EI_NIDENT]; /* Magic number and other info */
    Yeah, and ? What is that “magic number” ? What does that vague “other info” mean ?

    Now, if I look around a bit, I discover that on most Unices, there’s an excellent documentation on this subject (and the ELF format in general) in the man page of elf. And at the same time, I understand why it’s not in the comment above : the explanation is just too lengthy for a comment, it’s better on some separate documentation like this man page.

    But then… what is this comment for exactly ? People who are not knowledgeable of the ELF format beforehand can’t do anything with it, and people who have the knowledge still need to keep the man documentation around. It seems to me that the one who put this comment here just wanted to have a comment on every member of the Elf32_Ehdr structure. This is exactly the way of thinking which I want to avoid.

    My philosophy about this is that comments should be useful to the reader or not be. If what you want to express requires 20 lines of comments, it’s best to write a separate doc instead, and either put no comment at all or a reference to that doc. If the comment is a different way to spell the variable’s name, I don’t need it either…

    I’ve tried to rewrite the thing to make this more clear.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s