Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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?

share|improve this question
2  
It might have been better to post this on codereview.stackexchange.com/questions –  FrustratedWithFormsDesigner Aug 29 '11 at 13:56
add comment

closed as not constructive by Walter, FrustratedWithFormsDesigner, Mark Trapp Aug 29 '11 at 20:55

As it currently stands, this question is not a good fit for our Q&A format. We expect answers to be supported by facts, references, or expertise, but this question will likely solicit debate, arguments, polling, or extended discussion. If you feel that this question can be improved and possibly reopened, visit the help center for guidance.If this question can be reworded to fit the rules in the help center, please edit the question.

15 Answers

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?

share|improve this answer
    
Maybe if you keep the comments on the same line, but add some whitespace between the opening brace and the comment... –  djeidot Aug 29 '11 at 14:55
1  
That would help, but then, why not just make a line break? –  Malfist Aug 29 '11 at 15:53
1  
+1, don't mess with my visual brace scanning please –  Wyatt Barnett Aug 29 '11 at 18:11
add comment

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.

share|improve this answer
add comment

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.
}
share|improve this answer
    
I hope people don't take away that // self-commenting is something that should appear in their code. :-) –  benzado Aug 29 '11 at 19:05
    
@benzado: good catch -- the correct word is "self-documenting" :-) –  Jay Elston Aug 29 '11 at 19:51
    
thanks, guys ;) –  Konrad Morawski Aug 29 '11 at 20:24
1  
Well, I was just being snarky: if I saw that an author took the time to write a comment that merely declared the code to be self-documenting, I'd be somewhat annoyed. (In the context of your answer, there's nothing wrong with it.) –  benzado Aug 29 '11 at 21:33
add comment

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.

share|improve this answer
    
+1 for touching on the functional implications of this style –  CamelBlues Aug 29 '11 at 18:15
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
    
Code colorization helps a lot, in normal IDE I can quickly see only comments, or only code. You can't see that here. –  Coder Aug 29 '11 at 14:15
1  
@Coder: indeed, but you are relying only on code coloring, annulling the power of padding and empty lines between different portions of code. Why don't use them together for maximum visive intuition? –  Jose Faeti Aug 29 '11 at 14:17
    
Yes, but that means vertical expansion, which is not great on 16:9 screens. But this is only my opinion, and that's why I asked. –  Coder Aug 29 '11 at 14:23
    
@Coder: that's where portrait-oriented 1080p monitors (or more) come in hand (or so they say, never tried yet :) –  Jose Faeti Aug 29 '11 at 14:25
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
add comment

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

share|improve this answer
    
Why not? –  Anna Lear Aug 29 '11 at 13:31
    
@Anna: sorry, didn't understand what you mean... –  woliveirajr Aug 29 '11 at 13:32
    
Sorry, took me 3 tries to leave a comment. :) What I mean is, why don't you like to mix comments and code? Can you expand your answer a bit? –  Anna Lear Aug 29 '11 at 13:33
add comment

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.

share|improve this answer
add comment

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);
}
share|improve this answer
add comment

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);
}
share|improve this answer
add comment

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

share|improve this answer
add comment

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())
share|improve this answer
5  
Two words come to mind: Maintenance Nightmare –  ChaosPandion Aug 29 '11 at 14:32
    
ChaosPandion, I've seen and used this particular case in ASP code, mostly because the way it was structured required some pretty large code blocks between brackets and this actually helped understanding which condition was closed. It was helpful, but you are right that ultimately this is a maintenance nightmare and not really recommendable. –  Anne Schuessler Aug 29 '11 at 14:47
    
I actually started doing that sort of commenting because of the maintenance nightmares I inherited from other developers who would have several pages were of stuff enclosed in one condition. Yes, they should have refactored, but they didn't. With this, you at least know what the programmer intended. –  jwernerny Aug 29 '11 at 15:52
2  
All these comments are useless, except "Make apple sauce", and that should be eliminated and a "MakeAppleSauce" method created. –  kevin cline Aug 29 '11 at 17:18
2  
@jwernerny -- be proud of the downvotes -- good satire is difficult to recognize. –  Jay Elston Aug 29 '11 at 19:55
show 2 more comments

Not the answer you're looking for? Browse other questions tagged or ask your own question.