Stylistic Question: Use of White Space

Answer 1

It doesn't matter.

1) Develop a style that is your own. Whatever it is that you find easiest and most comfortable, do it. Try to be as consistent as you can, but don't become a slave to consistency. Shoot for about 90%.

2) When you're modifying another developer's code, or working on a group project, use the stylistic conventions that exist in the codebase or that have been laid out in the style guide. Don't complain about it. If you are in a position to define the style, present your preferences but be willing to compromise.

If you follow both of those you'll be all set. Think of it as speaking the same language in two different ways. For example: speaking differently around your friends than you do with your grandfather.

Answer 2

It's not petty to make pretty code. When I write something I'm really proud of, I can usually take a step back, look at an entire method or class, and realize exactly what it does at a glance - even months later. Aesthetics play a part in that, though not as large of a part as good design. Also, realize you can't always write pretty code, (untyped ADO.NET anyone?) but when you can, please do.

Unfortunately, at this higher level at least, I'm not sure there are any hard rules you can adhere to to always produce aesthetically pleasing code. One piece of advice I can offer is to simply read code. Lots of it. In many different frameworks and languages.

Answer 3

I minimize white space. I put the main comment block above the code block and Additional end of line comments on the Stuff that may not be obvious to another dveloper. I think you are doing that already

Answer 4

I find code with very little whitespace hard to read and navigate in, since I need to actually read the code to find logical structure in it. Clever use of whitespace to separate logical parts in functions can increase the ease of understanding the code, not only for the author but also for others.

Keep in mind that if you are working in an environment where your code is likely to be maintained by others, they will have spent the majority of their time looking at code that was not written by you. If your style distinctly differs from what they are used to seeing, your smooth code may be a speed bump for them.

Answer 5

My preferred style is probably anathema to most developers, but I will add occasional blank lines to separate what seem like appropriate 'paragraphs' of code. It works for me, nobody has complained during code reviews (yet!), but I can imagine that it might seem arbitrary to others. If other people don't like it I'll probably stop.

Answer 6

I like to break up logical "phrases" of code with white space. This helps others easily visualize the logic in the the method - or remind me when I go back and look at old code. For example, I prefer

reader.MoveToContent();
if( reader.Name != "Limit" )
 return false;

string type = reader.GetAttribute( "type" );
if( type == null )
 throw new SecureLicenseException( "E_MissingXmlAttribute" );

if( String.Compare( type, GetLimitName(), false ) != 0 )
 throw new SecureLicenseException( "E_LimitValueMismatch", type, "type" );

instead of

reader.MoveToContent();
if( reader.Name != "Limit" )
 return false;
string type = reader.GetAttribute( "type" );
if( type == null )
 throw new SecureLicenseException( "E_MissingXmlAttribute" );
if( String.Compare( type, GetLimitName(), false ) != 0 )
 throw new SecureLicenseException( "E_LimitValueMismatch", type, "type" );

The same break can almost be accomplished with braces but I find that actually adds visual noise and reduces the amount of code that can be visually consumed simultaneously.

Commens on code line

As for comments at the end of the line - almost never. The're not really bad, just easy to miss when scanning through code. And they clutter up the line taking away from the code making it harder to read. Our brains are already wired to grok line by line. When the comment is at the end of the line we have to split the line into two concrete concepts - code and comment. I say if it's important enough to comment on, put it on the line proceeding the code.

That being said, I do find one or two line hint comments about the meaning of a specific value are sometimes OK.

Answer 7

I like to maximize the amount of code that can be seen in a window, so I only use a single blank line between functions, and rarely within. Hopefully your functions are not too long. Looking at your example, I don't like a blank line for an open brace, but I'll have one for a close. Indentation and colorization should suffice to show the structure.

Answer 8

The most important thing to remember is that when you join an existing code base (as you almost always will in your professional career) you need to adhere to the code style guide dictated by the project.

Many developers, when starting a project afresh, choose to use a style based on the Linux kernel coding-style document. The latest version of that doc can be viewed at http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/CodingStyle;h=8bb37237ebd25b19759cc47874c63155406ea28f;hb=HEAD.

Likewise many maintainers insist that you use Checkpatch before submitting changes to version control. You can see the latest version that ships with the Linux kernel in same tree I linked to above at scripts/checkpatch.pl (I would link to it but I'm new and can only post one hyperlink per answer).

While Checkpatch is not specifically related to your question about whitespace usage, it will certainly help you eliminate trailing whitespace, spaces before tabs, etc.

Answer 9

I use exactly the same amount of whitespace as you :) Whitespace before methods, before comment blocks. In C, C++ the brackets also provide some "pseudo-whitespace" as there is only a single opening/closing brace on some lines, so this also serves to break up the code density.

Answer 10

Code Complete, by Steve McConnell (available in the usual locations) is my bible on this sort of thing. It has a whole chapter on layout and style that is just excellent. The whole book is just chock full of useful and practical advice.

Answer 11

Your code is fine, just do what you (and others you might work with) are comfortable with.

The only thing I see wrong with some (inexperienced) programmers about whitespace is that they can be afraid to use it, which is not true in this case.

I did however notice that you did not use more than one consecutive blank line in your sample code, which, in certain cases, you should use.

Answer 12

Here is how I would refactor that method. Things can surely still be improved and I did not yet refactor the PiggyBack class (I just moved it to an upper level).

By using the Composed Method pattern, the code becomes easier to read when it's divided into methods that each do one thing and work on a single level of abstraction. Also less comments are needed. Comments that answer to the question "what" are code smells (i.e. the code should be refactored to be more readable). Useful comments answer to the question "why", and even then it would be better to improve the code so that the reason will be obvious (sometimes that can be done by having a test that will fail without the inobvious code).

public CommandThread[] buildCommandsForExecution() {
    CommandLine cLine = buildCommandLine();
    CommandThread command = buildCommandThread(cLine);
    initPiggyBack(command);
    return new CommandThread[]{command};
}

private CommandLine buildCommandLine() {
    CommandLine cLine = new CommandLine(getValue(OptionConstants.EXECUTABLE));
    // "--test" must be first, and bucket and file location must be last,
    // because [TODO: enter the reason]
    cLine.addOption(getOption(OptionConstants.TEST));
    for (Option regularOption : getRegularOptions()) {
        cLine.addOption(regularOption);
    }
    cLine.addOption(getOption(OptionConstants.BUCKET));
    cLine.addOption(getOption(OptionConstants.FILE_LOCATION));
    return cLine;
}

private List<Option> getRegularOptions() {
    List<Option> options = getAllOptions();
    options.removeAll(getNonRegularOptions());
    return options;
}

private List<Option> getAllOptions() {
    List<Option> options = new ArrayList<Option>();
    for (OptionBox optionBox : optionBoxes.values()) {
        options.add(optionBox.getOption());
    }
    return options;
}

private List<Option> getNonRegularOptions() {
    OptionConstants[] nonRegular = {
            OptionConstants.BUCKET,
            OptionConstants.FILE_LOCATION,
            OptionConstants.TEST,
            OptionConstants.EXECUTABLE,
            OptionConstants.MOUNT_LOCATION
    };
    List<Option> options = new ArrayList<Option>();
    for (OptionConstants c : nonRegular) {
        options.add(getOption(c));
    }
    return options;
}

private CommandThread buildCommandThread(CommandLine cLine) {
    GUIInteractiveCommand command = new GUIInteractiveCommand(cLine, console);
    command.addComponentsToEnable(enableOnConnect);
    command.addComponentsToDisable(disableOnConnect);
    if (isMountLocationSet()) {
        command.addComponentToEnable(mountButton);
    }
    return command;
}

private boolean isMountLocationSet() {
    String mountLocation = getValue(OptionConstants.MOUNT_LOCATION);
    return !mountLocation.equals("");
}

private void initPiggyBack(CommandThread command) {
    PiggyBack piggyBack = new PiggyBack();
    piggyBack.setConsole(console);
    command.setNextLink(piggyBack);
}

Answer 13

For C#, I say "if" is just a word, while "if(" is code - a space after "if", "for", "try" etc. doesn't help readability at all, so I think it's better without the space.

Also: Visual Studio> Tools> Options> Text Editor> All Languages> Tabs> KEEP TABS!

If you're a software developer who insists upon using spaces where tabs belong, I'll insist that you're a slob - but whatever - in the end, it's all compiled. On the other hand, if you're a web developer with a bunch of consecutive spaces and other excess whitespace all over your HTML/CSS/JavaScript, then you're either clueless about client-side code, or you just don't give a crap. Client-side code is not compiled (and not compressed with IIS default settings) - pointless whitespace in client-side script is like adding pointless Thread.Sleep() calls in server-side code.