|
I, for example, would never write the code like that. I can write an if without the {}, but I always put an extra line break after the call. And seeing two lines after the if simply looks wrong.
if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)
goto fail;
if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
goto fail;
goto fail;
if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
goto fail;
So, this kind of error is as ugly to me as this (in fact, even uglier as the excessive {} somewhat hide the problem):
if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)
{
goto fail;
}
if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
{
goto fail;
}
goto fail;
if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
{
goto fail;
}
But developers can also commit these errors (and by simply taking a looks without reading the code, I don't spot anything wrong):
if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)
{
goto fail;
}
if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
{
goto fail;
}
{
goto fail;
}
if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
{
goto fail;
}
if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)
{
goto fail;
}
if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
{
goto fail;
}
else
{
goto fail;
}
if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
{
goto fail;
}
modified 11-Dec-14 13:35pm.
|
|
|
|
|
Well, I won't write anything such as that either. But since this a real world example of infamous 'goto fail' bug obviously other people do.
In general I'd say that none of your samples with braces would've passed my code review, but I admit your first sample should attract review attention as well, it'll probably will attract enough attention from any reviewer. Still I'd say that with braces style the code like that most probably would not leave the developer and will be fixed before review
Banking establishments are more dangerous than standing armies. T.Jefferson
|
|
|
|
|
I don't care what way you do it as long as it is not:
if (something) {
DoA()
}
Yuck!
But seriously - if the biggest problem your project has is where people put or don't put the braces you are doing pretty good.
|
|
|
|
|
Paulo Zemek wrote:
I agree with you. And I am actually the kind of person that when has to modify something like:
if (something)
{
DoA();
DoB();
}
To only call a DoAB(), I will go there and kill the extra { and }. So, I have more work doing that, but I keep consistency.
So, it becomes:
if (something)
DoAB(); |
Are you serious? You re-factor and combine methods just so you can avoid braces and use a dangerous bad practice?
|
|
|
|
|
No, I don't refactor and combine methods... I was only giving an example. What's more probably is that I extract an entire block (with many calls) into a single method. But for the example I used two calls.
|
|
|
|
|
And why couldn't you write
if (condition)
{
this.DoThis();
}
else
{
this.DoThat();
} Now this code is totally unambiguous
Although I know some people who really hate this...
My blog[ ^]
public class SanderRossel : Lazy<Person>
{
public void DoWork()
{
throw new NotSupportedException();
}
}
|
|
|
|
|
That's not good enough...
if
(
condition
)
{
this
.
DoThis
(
)
;
}
else
{
this
.
DoThat
(
)
;
}
It's not properly formatted until there's only one token per line.
|
|
|
|
|
Oooooooo, look at how easy that is to maintain now!! :drool:
Jeremy Falcon
|
|
|
|
|
if (condition) { this.DoThis(); } else { this.DoThat(); }
You probably meant
Ian Shlasko wrote: It's not properly formatted until there's only one token per line in the entire application. I guess it's theoretically possible...
My blog[ ^]
public class SanderRossel : Lazy<Person>
{
public void DoWork()
{
throw new NotSupportedException();
}
}
|
|
|
|
|
Way back in my College days the Software Lecturer told us (the class) it wasn't possible to do the above. I proved her wrong by writing a short Pascal (I think) program with no returns all on one line, ah Youth
|
|
|
|
|
The end of the story -
"...and that's how I got to repeat introduction to programming!!"
|
|
|
|
|
Quite. The maturity of a programming language may (in part) be calculated by how many newlines are required. A proper language requires none.
|
|
|
|
|
PIEBALDconsult wrote: Quite. The maturity of a programming language may (in part) be calculated by how many newlines are required. A proper language requires none.
Just for making that statement, you should be forced to a maintain decade-old Perl CGI system.
|
|
|
|
|
Perl is a scripting language; not a programming language.
|
|
|
|
|
Yeah true, you really need to be pedantic about a joke?
|
|
|
|
|
No, but I really need to be demeaning of Perl.
|
|
|
|
|
Of course it is, or at least, it was. That's what we used to do when programming in APL. There were execution costs for each new line you used.
|
|
|
|
|
You guys need to investigate APL, a language described by one of my fellow students as "the ultimate in elegance"
Jim
|
|
|
|
|
Totally agree...but for a very spesific reason. I'm using a Braille display with a maximum of 40 chars per line, so it have a influence on my format preferences...
Seriously though,
if (condition)
{
Action1();
}
else
{
Action2();
}
is my real preference, and there's a plus to that, whenever the situation change to have more than one statement for the if test, the person who has to change it doesn't have to remember to go and fill in the {}.
|
|
|
|
|
That looks good to me
|
|
|
|
|
Too much whitespace/not enough whitespace. The "Great Debate" goes on.
Some programmers get paid by LOC, so they're induced to spread things out. Some programmers like to show how smart they are, so they try to condense everything to one line of cryptic functions. Other programmers heard "this" is the right way to do it.
For me, the key is "consistency". I want my code and all my programmers' code to look identical. It should be totally transparent who wrote the code. And the code needs to be clear, quickly comprehensible and understandable, yet concise.
I don't want to spend my time figuring out what John Doe was doing and maybe miss some key element when I'm under pressure to fix a problem or develop a new capability.
I tell my programmers that "I was around long before you came and I will be around long after you leave. Do it my way because I will have to maintain it later."
I just had to work on a one-liner script from another company that incorporated at least 12 function calls. Took me the better part of a week to figure out where the bug was. Yes it looked slick, but a week wasted is two weeks lost.
That's my thoughts--clear, comprehensible, concise.
|
|
|
|
|
I'm not lazy, but I'll still use the latter. There's no point in always having to use a bracket so I don't, unless it makes things easier to read. Which in your example it doesn't. Wasting space just makes a source code file longer anyway and harder to navigate. Concise wins, unless it's hard to read.
Jeremy Falcon
|
|
|
|
|
it doesn't make anything easier to read, and it definitely makes things harder to maintain.
|
|
|
|
|
Chris, you can't BS me, come on man. You honestly expect me to buy that not using brackets for a single-line if condition makes code harder to maintain? Seriously? Where's the joke icon?
Jeremy Falcon
|
|
|
|
|
Sure it's easy, until another developer adds a second line and forgets to add brackets.
I've been working in some source code that didn't use brackets for single statements. I introduced a few bugs by not adding them when I had to and I've been wondering more than once if the original developer REALLY meant not to add brackets....
if (condition)
DoThis();
else
DoThat();
DoAnotherThing(); is really weird to look at and at the very least makes you wonder if it was intended...
Especially if DoAnotherThing(); isn't properly in/outdented!
My blog[ ^]
public class SanderRossel : Lazy<Person>
{
public void DoWork()
{
throw new NotSupportedException();
}
}
|
|
|
|