Comment on style

P

pocmatos

Hi all,

While I was programming 5 minutes ago a recurring issue came up and
this time I'd like to hear some opinions on style. Although they are
usually personal I do think that in this case as also to do with making
the code easier to read.

Imagine a function returning void (for example) and it's body is a big
if with lots of special cases:

void foo(const MyClass &c) {
if(c.bar()) {
x();
} else if(c.foobar()) {
y();
} else if(c.mybar()) {
z();
}
}

The question is if you prefer to see like I've wrote it or with a
return; after a call to x(), and another one after a call to y(), etc
etc... ?

Cheers,

Paulo Matos
 
I

Ian Collins

Hi all,

While I was programming 5 minutes ago a recurring issue came up and
this time I'd like to hear some opinions on style. Although they are
usually personal I do think that in this case as also to do with making
the code easier to read.

Imagine a function returning void (for example) and it's body is a big
if with lots of special cases:

void foo(const MyClass &c) {
if(c.bar()) {
x();
} else if(c.foobar()) {
y();
} else if(c.mybar()) {
z();
}
}

The question is if you prefer to see like I've wrote it or with a
return; after a call to x(), and another one after a call to y(), etc
etc... ?
What purpose would the returns serve? You are already in an if else
structure.
 
A

Alf P. Steinbach

* (e-mail address removed):
Imagine a function returning void (for example) and it's body is a big
if with lots of special cases:

void foo(const MyClass &c) {
if(c.bar()) {
x();
} else if(c.foobar()) {
y();
} else if(c.mybar()) {
z();
}
}

The question is if you prefer to see like I've wrote it

No, you should always use proper indentation, like

void foo(const MyClass &c) {
if(c.bar()) {
x();
} else if(c.foobar()) {
y();
} else if(c.mybar()) {
z();
}
}
or with a
return; after a call to x(), and another one after a call to y(), etc
etc... ?

With exception safe code it doesn't matter technically. But with
respect to clarity it's proper only for precondition checking. Within
the meat of the function it can hide the structure of the code (of
course that assumes proper indentation to begin with), and it can make
maintenance more difficult, for example where you want to add some
common action at the end, so there it's a judgement call.

For the C language the story is different, and I think the best advice I
can give to a novice here is: if you do not understand the difference
between C and C++ wrt. early returns, then absolutely don't use them;
otherwise, use clarity of code as the main guideline.
 
P

Pep

Hi all,

While I was programming 5 minutes ago a recurring issue came up and
this time I'd like to hear some opinions on style. Although they are
usually personal I do think that in this case as also to do with making
the code easier to read.

Imagine a function returning void (for example) and it's body is a big
if with lots of special cases:

void foo(const MyClass &c) {
if(c.bar()) {
x();
} else if(c.foobar()) {
y();
} else if(c.mybar()) {
z();
}
}

The question is if you prefer to see like I've wrote it or with a
return; after a call to x(), and another one after a call to y(), etc
etc... ?

Cheers,

Paulo Matos

Yep, style is definitely personal.

Ian Collins has already asked what purpose would the returns serve as you
are already in an if/else block. I agree with his question and would point
out that you have said this a big function, so you need to consider whether
it would be helpful for another developer to come at a later time and have
to deal with multiple exit points when trying to solve a bug in the
routine, which I would say it is not.


Alf P. Steinbach has stated that you should use proper indentation which
again I agree with but as to whether proper indentation demands 2 spaces, 4
spaces or tabs is open to personal interpretation. Myself, I prefer tabs.

Now I differ from most people that follow the K&R style because I would
prefer the code writtens as

void foo(const MyClass &c)
{

if(c.bar())
{
x();
}
else if(c.foobar())
{
y();
}
else if(c.mybar())
{
z();
}

}

Which I find more readable foe one thing and find it easier to comment out a
block of code because

} else if(c.foobar()) {
y();
} else if(c.mybar()) {

requires you to edit the "} else if(c.foobar()) {" line to place the "else"
statement on a separate line in order to comment out the line or insert a
"/*" in the middle of the else line, whereas

}
else if(c.foobar())
{
y();
}
else if(c.mybar())

simply requires you to place a /* ahead of the "else if(c.foobar())" line
and a "*/" after the closing brace.

Similarly I prefer to see

else if(c.foobar())
{
y();
}

as opposed to

else if(c.foobar())
y();

As you say, style is personal.

Finally I hate single character variable names, even in example code, even
though I sometimes (rarely) use them myself :)

Cheers,
Pep.
 
M

marco

(e-mail address removed) ha scritto:
Hi all,

While I was programming 5 minutes ago a recurring issue came up and
this time I'd like to hear some opinions on style. Although they are
usually personal I do think that in this case as also to do with making
the code easier to read.

Imagine a function returning void (for example) and it's body is a big
if with lots of special cases:

void foo(const MyClass &c) {
if(c.bar()) {
x();
} else if(c.foobar()) {
y();
} else if(c.mybar()) {
z();
}
}

The question is if you prefer to see like I've wrote it or with a
return; after a call to x(), and another one after a call to y(), etc
etc... ?

Cheers,

Paulo Matos
From your post I understand you want to know the difference between the
following two cases:

case 1:
void foo(const MyClass &c) {
if(c.bar()) {
x();
} else if(c.foobar()) {
y();
} else if(c.mybar()) {
z();
}
}

case 2:
void foo(const MyClass &c) {
if(c.bar()) {
x();
return;
} else if(c.foobar()) {
y();
return;
} else if(c.mybar()) {
z();
return;
}
}

Am I write or not?
 
M

marco

marco ha scritto:
(e-mail address removed) ha scritto:
From your post I understand you want to know the difference between the
following two cases:

case 1:
void foo(const MyClass &c) {
if(c.bar()) {
x();
} else if(c.foobar()) {
y();
} else if(c.mybar()) {
z();
}
}

case 2:
void foo(const MyClass &c) {
if(c.bar()) {
x();
return;
} else if(c.foobar()) {
y();
return;
} else if(c.mybar()) {
z();
return;
}
}

Am I write or not?
of course in case #2 else "must" be removed!
 
B

benben

Now I differ from most people that follow the K&R style because I would
prefer the code writtens as

void foo(const MyClass &c)
{

if(c.bar())
{
x();
}
else if(c.foobar())
{
y();
}
else if(c.mybar())
{
z();
}

}

I'd agree and that's what I would do...except when there's only one
statement in the block (and it'll remain in one statement in the near
future.) I prefer:

if (c.bar()) x();
else if (c.foobar()) y();
else if (c.mybar()) z();

Regards,
Ben
 
P

Pep

benben said:
I'd agree and that's what I would do...except when there's only one
statement in the block (and it'll remain in one statement in the near
future.) I prefer:

if (c.bar()) x();
else if (c.foobar()) y();
else if (c.mybar()) z();

Regards,
Ben

Yep, I can go along with that though myself I would still write it as a
brace'd block of code :)

Cheers,
Pep.
 
H

Howard

benben said:
I'd agree and that's what I would do...except when there's only one
statement in the block (and it'll remain in one statement in the near
future.) I prefer:

if (c.bar()) x();
else if (c.foobar()) y();
else if (c.mybar()) z();

The problem with that style is that many IDE's debuggers have trouble with
allowing you to step through that code nicely. I often set a breakpoint on
the action following the if, so that I can quickly see when the condition
succeeds. That's not possible if I can't set a breakpoint on it because
it's on the same line as the if. So... I'd put the action on the line
following the if, like this:

if (a)
doa();
else if b
do(b);

(Also, sometimes I'm not positive that I'll never add a second line of
action, or I get lost in what else goes with what if, so I end up adding
back those curly braces anyway. :))

-Howard
 
A

Aaron Graham

Which I find more readable foe one thing and find it easier to comment out a
block of code because

} else if(c.foobar()) {
y();
} else if(c.mybar()) {

requires you to edit the "} else if(c.foobar()) {" line to place the "else"
statement on a separate line in order to comment out the line or insert a
"/*" in the middle of the else line, whereas

}
else if(c.foobar())
{
y();
}
else if(c.mybar())

simply requires you to place a /* ahead of the "else if(c.foobar())" line
and a "*/" after the closing brace.

Or...you could just do this:

// } else if(c.foobar()) {
// y();
} else if(c.mybar()) {

The problem with the "every brace gets its own line" philosophy is that
reading the code requires a lot more scrolling. It's extremely
aggrivating trying to figure out a piece of code where half of all the
lines are just braces, because there's never very much information on
one page.

I find that It's easy to tell how often someone has to read/debug other
peoples' code by how he structures his his own code. If he uses a
separate line for every brace, tabs all over the place (or worse yet,
spaces mixed with tabs), hungarian notation, etc., then he probably
hasn't spent much time looking at someone else's code.

And, no offense, but generally, people who code in a vacuum like that
don't often write good code. In fact, the best coders I know don't get
to write their own code very often. They spend most of their time
fixing someone else's bugs, since they're the only ones who can.
Finally I hate single character variable names, even in example code, even
though I sometimes (rarely) use them myself :)

There's nothing wrong with it if its scope is very localized. If I can
see what 'x' is simply by looking at the line of code above where it's
used, there's no reason to give 'x' a long name; it's just clutter.

Aaron
 
S

shaun roe

Hi all,

While I was programming 5 minutes ago a recurring issue came up and
this time I'd like to hear some opinions on style. Although they are
usually personal I do think that in this case as also to do with making
the code easier to read.

Imagine a function returning void (for example) and it's body is a big
if with lots of special cases:

void foo(const MyClass &c) {
if(c.bar()) {
x();
} else if(c.foobar()) {
y();
} else if(c.mybar()) {
z();
}
}

The question is if you prefer to see like I've wrote it or with a
return; after a call to x(), and another one after a call to y(), etc
etc... ?

Cheers,

Paulo Matos

definitely as you wrote it, and with the braces, in case you decide you
need an extra action in there later. On the other hand, a lot of such
structures (maybe not your particular case) can be written as a find in
a map of function pointers, which can clarify the body of the code.
 
G

Gavin Deane

Aaron said:
The problem with the "every brace gets its own line" philosophy is that
reading the code requires a lot more scrolling. It's extremely
aggrivating trying to figure out a piece of code where half of all the
lines are just braces, because there's never very much information on
one page.

And yet ...

The problem with the "every brace on the code line" philosophy is that
reading the code requires a lot more scrutiny. It's extremely
aggravating trying to figure out a piece of code where all the vertical
whitespace has been removed, because there's always too much
information on one page.

It really is just a matter of personal preference. What makes it harder
for you to read the code makes it easier for me, and vice versa. I
don't think that says anything about our relative abilities as
programmers.

Gavin Deane
 
R

roberts.noah

Gavin said:
And yet ...

The problem with the "every brace on the code line" philosophy is that
reading the code requires a lot more scrutiny. It's extremely
aggravating trying to figure out a piece of code where all the vertical
whitespace has been removed, because there's always too much
information on one page.

It really is just a matter of personal preference. What makes it harder
for you to read the code makes it easier for me, and vice versa. I
don't think that says anything about our relative abilities as
programmers.

I find it easier to establish where a block is as opposed to the the
statement that 'starts' it.
if (xxx) {
code
codesnthasonethua
aoeuaeou
}

is rather agrivating to read; the separation betweer "if..." and "code"
just isn't adiquate. In coding standards where I have to put the { on
the line with the if I go ahead and hit enter twice.

If you really want to save vertical space why not write it all on one
damn line anyway?

if (xxx) { code; codesntoheustnah; aeouaoeu; }

There, no waste of vertical space :p
 
A

Aaron Graham

The problem with the "every brace on the code line" philosophy is that
reading the code requires a lot more scrutiny. It's extremely
aggravating trying to figure out a piece of code where all the vertical
whitespace has been removed, because there's always too much
information on one page.

I used to think this way, too, way back when I first started using C
code. But I found that once I got used to reading it, code with less
vertical whitespace could be grokked more quickly. (Notice I said
"less whitespace", not "no whitespace" -- all other good style
guidlines -- such as indentation --still apply.)
It really is just a matter of personal preference. What makes it harder
for you to read the code makes it easier for me, and vice versa. I

No, not really. I read code written in both styles all the time. I
can read "every brace gets its own line" code just as well as anyone
else, but the physical act of having to scroll around and the visual
discontinuity is an impediment to an even quicker understanding. How
could it not be? Can you at least agree that it's faster to glance
around a single page than it is to scroll around through several?

Aaron
 
B

BigBrian

The problem with the "every brace gets its own line" philosophy is that
reading the code requires a lot more scrolling.

So what? Most environments support scrolling just fine.
It's extremely
aggrivating trying to figure out a piece of code where half of all the
lines are just braces, because there's never very much information on
one page.

Aggravating maybe to you, but not to everybody.
I find that It's easy to tell how often someone has to read/debug other
peoples' code by how he structures his his own code. If he uses a
separate line for every brace, tabs all over the place (or worse yet,
spaces mixed with tabs), hungarian notation, etc., then he probably
hasn't spent much time looking at someone else's code.

This is your opinion, and is a generalization which will not always
hold. It also gives the perception that you will label code as bad
just because the coder uses tabs or hungarian notation.
And, no offense, but generally, people who code in a vacuum like that
don't often write good code.

How you format your code doesn't make it good or bad. The quality is
determined by if it meets the requirements, doesn't cause undefined
behavior, easy to maintain,...
In fact, the best coders I know don't get
to write their own code very often. They spend most of their time
fixing someone else's bugs, since they're the only ones who can.

Yes, I can see that.
 
G

Gavin Deane

Aaron said:
I used to think this way, too, way back when I first started using C
code. But I found that once I got used to reading it, code with less
vertical whitespace could be grokked more quickly. (Notice I said
"less whitespace", not "no whitespace" -- all other good style
guidlines -- such as indentation --still apply.)

Actually you said "less vertical whitespace", not "less whitespace",
which implies that you might not accept "no whitspace". I just happen
to disagree that, of the styles presented, code with less vertical
whitespace is easier to read, because I find it cluttered. But
'cluttered' is still a subjective term. Where variable names are
concerned, my personal preference is very much towards the longer, more
verbose end of the scale. Other people might regard that as clutter and
prefer to accept a certain level of abbreviation.
No, not really. I read code written in both styles all the time. I
can read "every brace gets its own line" code just as well as anyone
else, but the physical act of having to scroll around and the visual
discontinuity is an impediment to an even quicker understanding.

I suggest that vertical whitespace style is a matter of personal
preference. You contend that it is something else. But the only
evidence you can provide is your own subjective experience.
How
could it not be? Can you at least agree that it's faster to glance
around a single page than it is to scroll around through several?

Not if the time it takes to comprehend vertically compressed code is
greater than the time it takes to scroll the page back and forth. I am
also familiar with both styles. It so happens that the style I find
easier to read is the one you find harder to read. I still think that
says nothing whatsoever about our relative skills as programmers.

Gavin Deane
 
B

Baxter

Hi all,

While I was programming 5 minutes ago a recurring issue came up and
this time I'd like to hear some opinions on style. Although they are
usually personal I do think that in this case as also to do with making
the code easier to read.

Imagine a function returning void (for example) and it's body is a big
if with lots of special cases:

void foo(const MyClass &c) {
if(c.bar()) {
x();
} else if(c.foobar()) {
y();
} else if(c.mybar()) {
z();
}
}

The question is if you prefer to see like I've wrote it or with a
return; after a call to x(), and another one after a call to y(), etc
etc... ?


My preference is:

void foo(const MyClass &c)
{
if (c.bar()) {
x();
}
else if (c.foobar()) {
y();
}
else if (c.mybar()) {
z ();
}
}

This is a Modified K&R, and (IIRC) used by the BCD.

Note the following:
- only one brace on a line. Brace, opening or closing, is followed by c/r
- body of conditionals are indented (for easy reading)
- opening brace for _function_ is on line by itself (largely for historical
reasons)
- leading brace on same line as conditional,
-- reduces visual redundancy of implied "begin",
-- opening brace is _always_ related to conditional. If on same line,
you cannot accidentally insert code.
-- reduces vertical whitespace improving readability
-- allows easy insertion of new code, or deletion of old code block.
-- gives greater correspondence with English:
if (conditional)
DoThis; // body of conditional
end conditional
-- presents no known problems with any debugger.

YMMV
 
A

Aaron Graham

A lot of things are a matter of preference but many things are not.
Usually when I'm faced with the task of choosing a consistent style
guideline or tool or environmental preference, I first consider the
following:

1) If I was equally "fluent" in both styles, would one of them be
superior in any respect?
2) Is it too difficult or are there other barriers to becoming "fluent"
in either style?

Sometimes, (1) turns out to be "no", or more often "nothing comes to
mind", in which case it certainly is a matter of preference. Sometimes
(2) turns out to be "yes", in the sense that one style is more
difficult to become accustomed to, or because one requires a
substantial amount of mental faculty that the other doesn't.

Take, for instance, the hypothetical argument of Windows Notepad vs.
Emacs as a development environment The answer to question (1) is "yes"
because emacs is more powerful and will help you be a more productive
developer than Windows Notepad will, given that you know them equally
well. The answer to (2) is "yes" because emacs takes more effort to
get to know and learn to use well. This is an extreme example, but it
shows that Windows Notepad vs. Emacs is not so much a matter of
preference as it is a matter of effort or learning curve. Nobody who
knows emacs well will voluntarily use Windows Notepad for code
development. You'll never hear someone say "I know how to use emacs,
but I like Windows Notepad better." The argument of C vs. C++ or GUI
vs. CLI is somewhat similar.

Now, apply these questions to brace styles: The answer to (1) is
"yes," being able to fit more code on a single page is certainly
better, readability notwithstanding. Someone who is _equally_fluent_
in both styles will find that he can be more productive if he's not
hampered as much by the physical act of scrolling. Note that this
point is NOT subjective. People who could scroll around faster than
they could move their eyes would probably make their windows only a few
lines tall, but I can't be sure because I don't know anyone like that.
So it comes down to (2) which is the part of the question that
addresses readability, and is the only part that is subjective. Since
it took me a little longer to learn to read vertically compressed code
well, I can admit that it takes more experience and some adjustment for
your mind to get over the perception of the code being "cluttered", but
I don't think it was _too_ difficult.

So I would have answered "no" to (2) but maybe some people just can't
get their minds over the "cluttered" aspect. I'm pretty sure that
people who consistently write code in one-line-per-brace style would
have difficulty ever getting over it. (As an analogy, it's much harder
to learn a language well unless you speak or write it.) So maybe
you're right in one aspect -- it takes an undesirable amount of effort
for some people to learn to read vertically compressed code as well.
Some people work for draconian companies that enforce a certain style,
so they never really get much of chance to learn to write or read
differently-styled code. I've worked for and have dealt with such
companies before. (I even worked with a company that required
developers to mix spaces and tabs in their indentation, and to use a
brace style that I've _never_ seen anyone else use. I personally think
it was more of an obfuscation technique than anything else.)

As an extreme, regarding code with NO non-essential whitespace (except
for word-wrap at column 80 or so), the answer to (1) is still "yes",
but the answer to (2) is also a resounding "yes", since the amount of
effort required to read it is excessive. But once again, it's not as
much a matter of personal preference as it is effort or mental faculty.
Not if the time it takes to comprehend vertically compressed code is
greater than the time it takes to scroll the page back and forth. I am
also familiar with both styles. It so happens that the style I find
easier to read is the one you find harder to read. I still think that
says nothing whatsoever about our relative skills as programmers.

Sorry, I shouldn't have said anything about programming skill. I was
just stating a strong observed correlation, but there are certainly
outliers, and I don't doubt your programming abilities.

This really is a silly topic to have so much debate about. I'll be off
now.

Aaron
 
J

Jim Langston

Hi all,

While I was programming 5 minutes ago a recurring issue came up and
this time I'd like to hear some opinions on style. Although they are
usually personal I do think that in this case as also to do with making
the code easier to read.

Imagine a function returning void (for example) and it's body is a big
if with lots of special cases:

void foo(const MyClass &c) {
if(c.bar()) {
x();
} else if(c.foobar()) {
y();
} else if(c.mybar()) {
z();
}
}

The question is if you prefer to see like I've wrote it or with a
return; after a call to x(), and another one after a call to y(), etc
etc... ?

Cheers,

Paulo Matos

From structured programming, a function is supposed to only have one exit
point. Meaning the way you have it is correct. The justification for this
is that it's much easier to understand the flow of a function, top/down.

Say your function was in fact like this:

void foo(const MyClass &c)
{
if (c.bar())
{
x();
return;
}
else if (c.foobar())
{
y();
return;
}
else if ( c.mybar() )
{
z();
return;
}

zz();

}

Which I've actually seen in code. If someone was searching code to see
where zz() was called and the if statements were actually many lines, one
could presume wrongly that zz() is always called in the function foo. In
fact it's only called if the other conditions are not met.

Then comes the fun when, say, else if ( c.foobar() ) doesn't have a return
in it's block. So both y() and zz() are called in that instance, another
case I've seen. Then it becomes even more confusing when zz() is called and
isn't.

Of course the "proper" way would be to get rid of the returns, and wrap the
call to zz() in an else statement, then it once again becomes clear. And
have the c.foobar() also call zz() if it needs to.

Top down coding requires there only be one exit point for a function, but as
I think we all know, rules are made to be broken. There are some situations
it just makes the function much clearer as to what it's doing to have a
return statement other at the bottom of the function, but this should be the
exception rather than the rule in top down coding.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,770
Messages
2,569,584
Members
45,075
Latest member
MakersCBDBloodSupport

Latest Threads

Top