What do you think of opening brace comments in source code? [closed]

Question

I have developed a habit of writing comments in my code by putting the comments on the same line as the opening brace, after the brace. I've found that this saves vertical space. It also leaves a hint why something was done, but I'm wondering if it's readable for others.

Example:

void DoSomeInterestingImageManipulation(char *pImage)
{//This will convert the image to formatABC which allows x% space savings for storage
    if(pImage && pImage[0] == 0xFF)
    {//Process the extra case where image internal format needs decompression
        ++pImage;
        //...
        //...
        //...
    }

    //Proceed normally
    *pResult = Foo(pImage);
}

Do you consider it easier to read or harder to read?

Answer 1

I would not recommend that style as it makes it difficult to scan for braces.

void DoSomeInterestingImageManipulation(char *pImage)
{//This will convert the image to formatABC which allows x% space savings for storage
    if(pImage && pImage[0] == 0xFF)
    {//Process the extra case where image internal format needs decompression
        ++pImage;
        //...
        //...
        //...
    }

    //Proceed normally
    *pResult = Foo(pImage);
}

Look at the end braces and tell me where the block begins.
Now do the same for this one:

void DoSomeInterestingImageManipulation(char *pImage)
{
    //This will convert the image to formatABC which allows x% space savings for storage
    if(pImage && pImage[0] == 0xFF)
    {
        //Process the extra case where image internal format needs decompression
        ++pImage;
        //...
        //...
        //...
    }

    //Proceed normally
    *pResult = Foo(pImage);
}

Can you tell me where the start more easily?

Answer 2

It feels weird. There's no problem with it. You could also do:

void foobar() {
    // comment explaining function
    ...
}

And you get the same number of lines just by putting the opening brace on the same line as the function name.

Answer 3

Personally, I have never seen much point in "saving vertical space" for its own sake. We have plenty of vertical space. There are applications out there that use milions of lines of vertical space.

If some method gets too big vertically, it means it should be split. Putting more and more stuff in same lines is not a solution, it's just hiding the problem. The readability of the style you suggested is poor in my opinion.

Instead of:

if(pImage && pImage[0] == 0xFF)
{//Process the extra case where image internal format needs decompression
    ++pImage;
    //...
    //...
    //...
}

Just:

if(pImage && pImage[0] == 0xFF)
{
    processDecompressableImage(pImage); // separate method. self-documenting.
}

Answer 4

I think it's readable but weird. Whenever I see something next to a brace, I expect it to be part of the body. Now a // or a /* gives the comment away, but it doesn't feel natural.

Another big issue is ineffective debugging. Consider this:

// code

for () { /* comment
    end comment */
loop
} */

//code 

If you decide that you want to comment out the loop to test the effect, what you'll end up doing is this:

/* for () { /* comment
        end comment */
    loop
    } */

and the loop remains. Now this is kind of a strange example, but the issue is there.

In addition, you haven't established a concrete reason for the advantages of this style. I don't think "saving a line" counts.

Answer 5

Personally, I don't find your comment style readable. I prefer my comments that explain a block of code to be above the block of code. In languages that use curly braces, this is typically above the curly brace.

Also, I noticed from your comments that you are explaining a lot of "what", although this might just be in the sample. This typically goes out of date faster than the code since people will change the code but not the comments. Your comment about doing something because it saves space is a good comment, but the other ones, not so useful. Not having the comments where they don't explain rationale would save space and improve readability.

Answer 6

I've never seen such an unreadable piece of code.

All those comments between code lines will only get in the way. You should write self-documented code and perhaps considering a comment before the function.

Then, if you really have the need to have comments between code lines, at least write them before an important (or not so easy to understand) piece of code, in an empty line and with the same padding as the code below.

Edit

To clarify, even with syntax highlighting the comments look out of place to the code itself, because of missing paddings.

For a better readability, paddings and empty lines are great at separating different portions of code. Together with syntax highlighting, you obtain a greater code readability.

Answer 7

What about:

void DoSomeInterestingImageManipulation(char *pImage)
{
    convertToFormatAbc(pImage); // allows x% space savings.
    *pResult = Foo(pImage);
}

void convertToFormatAbc(char *pImage)
{
    if (internalFormatNeedsDecompression(pImage))
    {
        decompress(pImage);
    }
}

boolean internalFormatNeedsDecompression(char *pImage)
{
    return pImage && pImage[0] = 0xFF;
}

The point that I want to illustrate is that you should aim to have self-documenting code. That will require very few comments, if at all. If you need comment, always go for readability, not screen space savings.

In my opinion, comments above or at the beginning of a block of code simply means that this should be a method/class of its own.

Answer 8

Personally, I would write the same code as this:

///Converts the image to formatABC to save x% space
void DoSomeInterestingImageManipulation(char *pImage) {
    //Handle need for internal decompression
    if(pImage && pImage[0] == 0xFF) {
        ++pImage;
        //...
        //...
        //...
    }

    //Proceed normally
    *pResult = Foo(pImage);
}

If you have to comment what something does, then write it before that something. Some people prefer to just read your code and will only be distracted by your comments. Some might want to read your comments and not have to search them within you code.
Also, keep comments as sparse, short and clear as possible. If you have the feeling that you have to save some of the lines occupied by the comments, than consider making shorter comments or removing them entirely and choosing self-explanatory identifiers instead.

Answer 9

Your justifications make no sense to me. I'm not sure why you'd want to save vertical space unless you plan to print it out all the time (and why would you do that) because the net result is less legible code on the screen. I don't see that as productive. Secondly, how do you plan to avoid leaving a hint of how/why something was done without a comment of some description? It's not the location of the comment that gives that information, it's the content. It's possible to construct comments in a more legible manner than this and there are a number of examples posted already.

I don't know why you developed this habit - maybe it is something like a signature for you. Additionally, being used to it, you may find it easier to read now. However, I don't really find it particularly legible as code goes.

The short answer to your question is it's harder to read.

Answer 10

Don't like to mix comments and code. Even if the code is a single {.

For example, some IDEs helps you by greying out lines that are commented. When you mix code and comment, you can loose this code-coloring.

Another point: I prefer to use one comment-symbol per line. In Java, for example, I prefer to use lots of //, one each line, instead of /* and */.

This way, when debuging, I can use the /* */ to comment the code that'll be left out without having to see where I'm finishing explanation-comments

Answer 11

Although it's just my taste, I personally like the extra space (and the clear visual structure) I get from the opening brace. That's also why I always hated the brace directly after the if/loop/function head. And I also don't like comments in the same line as code, because it again violates the clear structuring a bit.

Answer 12

The biggest problem in your code snippet is comments. It looks ugly and unreadable in any case. place comments to separated line.

void DoSomeInterestingImageManipulation(char *pImage)
{ 
    //This will convert the image to formatABC which allows x% space savings for storage
    if(pImage && pImage[0] == 0xFF)
    {
        //Process the extra case where image internal format needs decompression
        ++pImage;
        //...
        //...
        //...
    }

    //Proceed normally
    *pResult = Foo(pImage);
}

is much better. I used this "style" in past but now I use better one)

void DoSomeInterestingImageManipulation(char *pImage) { 
    //This will convert the image to formatABC which allows x% space savings for storage
    if(pImage && pImage[0] == 0xFF) {
        //Process the extra case where image internal format needs decompression
        ++pImage;
        //...
        //...
        //...
    }

    //Proceed normally
    *pResult = Foo(pImage);
}

Answer 13

I don't mind it so much, but I would like to see some indentation there, so the braces still easily stand out when scanning over code:

void DoSomeInterestingImageManipulation(char *pImage)
{   //This will convert the image to formatABC which allows x% space savings for storage
    if(pImage && pImage[0] == 0xFF)
    {   //Process the extra case where image internal format needs decompression
        ++pImage;
        //...
        //...
        //...
    }

    //Proceed normally
    *pResult = Foo(pImage);
}

Answer 14

I actually like to keep that extra space empty, but perhaps it's just a matter of getting used to.

Answer 15

I like the style, and would actually go one step further and put a comment on the closing brace to indicate what it is closing.

The only major change I would make is to make sure the opening brace is a little more obvious by putting space after it.

while (!fruitList.isEmpty())     // while there is fruit
{                                // ... do something with it
    Fruit fruit = fruitList.takeFirst();

    switch (fruit)      
    {
    case apple:
        MakeAppleSauce();
        break;

    case banana:
        MakeBananaBread();
        break;

    case grape:
        MakeWine();
        break;

    default:
        // Don't do anything (for now)
    }   // switch (fruit)
}       // while (!fruitList.isEmpty())