Code Neatness
-
So somewhere on the internet, I pointed out to a newbie that his code was pretty messy and that he should start by consolidating some of his variable declarations to the beginning of the program (which was consisted of only a main(…) function/method/entry point and a lot of improperly formatted code). In walks another forum member and criticizes me for my comment saying that "it's a feature of the C++ language to be able to declare variables anywhere you need it." I was obviously pissed at that comment showing his utter disregard for code neatness and readability (and likely a malicious attack on my intelligence). I'm trying to seek agreement… so was I right when I said so?
So the creationist says: Everything must have a designer. God designed everything. I say: Why is God the only exception? Why not make the "designs" (like man) exceptions and make God a creation of man?
Try looking up Scope and Span in 'Code Complete'. Having a lot of distance between the variable declaration and its last usage technically increases the complexity of the code. It also decreases the readability. Having a large span/scope increases the chances of misuse of a variable, and therefore bugs. Keeping a variables scope down to a mimimum also relates to good practises like information hiding. You were wrong. It may seem more readable to you because that is what you are used to. Try programming with reduced span/scope for a few days, you will soon switch over. regards, Andrew
-
You're still talking readability.... I'm referring to taking advantage of the scoping of variables within functions in C++. Besides that, to me, it's more readable to me if the declarations are close by. Mark
Mark Salsbery Microsoft MVP - Visual C++ :java:
Mark Salsbery wrote:
I'm referring to taking advantage of the scoping of variables within functions in C++.
I think Ri addresses this (italics added by me):
Ri Qen-Sin wrote:
my style is to declare it as close to the beginning as possible while keeping all the local variables deifnitions in the most local scope.
So, if I'm interpreting this correctly, Ri groups the declaration of (new) variables used within a block at the beginning of that block, taking advantage of block scoping. On the other hand, Ri's orginal advice:
Ri Qen-Sin wrote:
start by consolidating some of his variable declarations to the beginning of the program
in isolation sounds like a very simplistic/outdated approach but might well make sense in the context of the code he was commenting on in the first place:
Ri Qen-Sin wrote:
which was consisted of only a main(…) function/method/entry point and a lot of improperly formatted code
Ri, I think this should be a valuable lesson for you: By all means, recommend that scrappy looking code be cleaned up & formatted but never give coding style advice in a public forum! Leave that up to the individual or you open yourself up for a world of pain...X| At the very least, if you must give code style advice, be certain to qualify it with "it's my personal preference to..." :)
T-Mac-Oz
-
AmazingMo wrote:
Do you still use Hungarian notation? Some times it's better to just let go.
The original concept of Hungarian Notation was actually quite different from M$s butchery of it. I forget the actual words used to define the concept but the prefix was really meant to indicate the usage of the variable, not its data-type. E.g.
//M$: DWORD dwVar1, dwVar2; // M$ uses prefix to denote type // not very useful in the grand scheme of things, // since the compiler picks up type mismatches // What is the variable used for? // You have to scan the code to find out for (dwVar1 = 0UL; dwVar1 ... { for (dwVar2 = 0UL; dwVar2 ... //Original Hungarian: DWORD outerIterVar, innerIterVar; // Original Hungarian uses prefix to denote usage (e.g. "outer iterator"), // giving the programmer an idea of the intended usage of the variable // without having to scan the code for (outerIterVar = 0UL; outerIterVar ... { for (innerIterVar = 0UL; innerIterVar ...
Having worked with MFC and WinAPI for so many years, it came as a surprise to learn this but I saw the sense in it right away & have tried to "do it properly" ever since. Unfortunately, old habits die hard... sigh.T-Mac-Oz
T-Mac-Oz wrote:
The original concept of Hungarian Notation was actually quite different from M$s butchery of it. I forget the actual words used to define the concept but the prefix was really meant to indicate the usage of the variable, not its data-type. E.g.
I recall reading an example that used a prefix 'c' to signify a count rather than 'i' for integer. So, the way it was intended to be used was:
Int32 cRows;
But people started using it as:Int32 iRows; // the count of rows is an integer, so, ah, I'll prefix it with i
Although the more readable way would be to declare it as:Int32 rowCount;
~ Arun -
So somewhere on the internet, I pointed out to a newbie that his code was pretty messy and that he should start by consolidating some of his variable declarations to the beginning of the program (which was consisted of only a main(…) function/method/entry point and a lot of improperly formatted code). In walks another forum member and criticizes me for my comment saying that "it's a feature of the C++ language to be able to declare variables anywhere you need it." I was obviously pissed at that comment showing his utter disregard for code neatness and readability (and likely a malicious attack on my intelligence). I'm trying to seek agreement… so was I right when I said so?
So the creationist says: Everything must have a designer. God designed everything. I say: Why is God the only exception? Why not make the "designs" (like man) exceptions and make God a creation of man?
I used to do that (all huddled at the top) but now I declare as close to first-use as possible. Functions/methods shouldn't be so large/complicated that you need to huddle all variables at the top to make it readable.
regards, Paul Watson Ireland & South Africa
Fernando A. Gomez F. wrote:
At least he achieved immortality for a few years.
-
Ri Qen-Sin wrote:
I'm trying to seek agreement… so was I right when I said so?
It doesn't matter what "we" think. If your idea of pretty code requires consolidated variable declarations then stick with it and provide your own validation.
A human being should be able to change a diaper, plan an invasion, butcher a hog, conn a ship, design a building, write a sonnet, balance accounts, build a wall, set a bone, comfort the dying, take orders, give orders, cooperate, act alone, solve equations, analyze a new problem, pitch manure, program a computer, cook a tasty meal, fight efficiently, die gallantly. Specialization is for insects. - -Lazarus Long
I used to be "top of the block" declarer, that is, until I started to do more code review/maintenance. Now I wish the declares were all JIT. The REAL messy code problem is: Long code lines requiring horizontal scrolling. MH
-
I disagree with you - declare as you need them
Christian Graus - Microsoft MVP - C++ "also I don't think "TranslateOneToTwoBillion OneHundredAndFortySevenMillion FourHundredAndEightyThreeThousand SixHundredAndFortySeven()" is a very good choice for a function name" - SpacixOne ( offering help to someone who really needed it ) ( spaces added for the benefit of people running at < 1280x1024 )
On topic: Declare them as you need them. Off topic: Christian, that's possibly the funniest signature I've ever seen! :laugh:
Mike Devenney
-
I used to be "top of the block" declarer, that is, until I started to do more code review/maintenance. Now I wish the declares were all JIT. The REAL messy code problem is: Long code lines requiring horizontal scrolling. MH
-
Code readability (and set standards) is fundamental to developing consistent, maintainable code. Picking a good, comprehensive standard way of coding means less chance of errors appearing because different parts of the code will be doing things in a similar fashion so there's less chance of conflict. Having one standard of writing code means that when you are browsing code it will all look the same so errors through incorrect implementation will be easier and faster to spot. It's not where you declare your variables that matters. It's how readable and maintainable your code is. Personally I prefer to declare my variables as close to their first use as possible. Other like them all sitting, huddled, at the top of their functions, too scared to mingle with the rest of the crowd. It's kind of like the kitchen of the function.
cheers, Chris Maunder
CodeProject.com : C++ MVP
According to Scott Meyers, author of "Effective C++: 55 Specific Ways to Improve Your Programs and Designs" you should "Postpone variable definitions as long as possible" (p 113) He actually dedicates a complete chapter to the subject. The reasoning for posponing declarations is to avoid incurring unnecessary construction overhead. This mostly applies to functions that have branching that may prevent all code segments from being executed. I personally don't think that declaring variables in the code body contributes to or against readability.
-
I am already using two monitors, so I am talking about REALLY long lines! :-D
-
Ravi Bhavnani wrote:
If the code loops a million times, I think we have a much bigger problem.
Usually the problem that requires millions of iterations in it's solution :-D Search engine query? How many hash buckets would you need to eliminate child node searches altogether? "grep"ing a large text (or set of) files. SQL query on a large database (somebody's got to write the query engine!) Sure, these are examples of stuff that typically has a multitude of pre-existing, mature solutions but a) they're just examples to show that there are domains where millions of iterations can be reasonably expected b) Somebody's got to maintain it!
T-Mac-Oz
The good practice is “Lazy Evaluation” – declaring as late as possible, and keep it within the scope only (hide it from outside). If you declare everything in the beginning of a function, you probably will forget to clean up the residual data before using it.
-
AmazingMo wrote:
Try it a million times or so and see what happens.
If the code loops a million times, I think we have a much bigger problem. :)
AmazingMo wrote:
most compilers will hoist the variable declaration out of the loop scope, but do you want to take the risk that the particular compiler that you're using will?
[edit] I can't imagine I'd use one that didn't do that as part of standard optimization! Code hoisting has been around for decades and is pretty mature technology. [/edit]
AmazingMo wrote:
You started programming in C, didn't you
No. I started programming before the first mainstream C compilers came to market. /ravi
My new year resolution: 2048 x 1536 Home | Music | Articles | Freeware ravib(at)ravib(dot)com
modified on Monday, March 10, 2008 2:17 AM
Ravi Bhavnani wrote:
can't imagine I'd use one that didn't do that as part of standard optimization! Code hoisting has been around for decades and is pretty mature technology.
If the object has a constructor or destructor, the C++ standard requires that it is destroyed and a new object created on every iteration of the loop. But the compiler will construct the object in the same space each time.
DoEvents: Generating unexpected recursion since 1991
-
So somewhere on the internet, I pointed out to a newbie that his code was pretty messy and that he should start by consolidating some of his variable declarations to the beginning of the program (which was consisted of only a main(…) function/method/entry point and a lot of improperly formatted code). In walks another forum member and criticizes me for my comment saying that "it's a feature of the C++ language to be able to declare variables anywhere you need it." I was obviously pissed at that comment showing his utter disregard for code neatness and readability (and likely a malicious attack on my intelligence). I'm trying to seek agreement… so was I right when I said so?
So the creationist says: Everything must have a designer. God designed everything. I say: Why is God the only exception? Why not make the "designs" (like man) exceptions and make God a creation of man?
Here's a free site for cleaning up your SQL...just paste your code in it and hit the Prettify! button.... www.simple-talk.com/prettifier/
-
AmazingMo wrote:
Do you still use Hungarian notation? Some times it's better to just let go.
The original concept of Hungarian Notation was actually quite different from M$s butchery of it. I forget the actual words used to define the concept but the prefix was really meant to indicate the usage of the variable, not its data-type. E.g.
//M$: DWORD dwVar1, dwVar2; // M$ uses prefix to denote type // not very useful in the grand scheme of things, // since the compiler picks up type mismatches // What is the variable used for? // You have to scan the code to find out for (dwVar1 = 0UL; dwVar1 ... { for (dwVar2 = 0UL; dwVar2 ... //Original Hungarian: DWORD outerIterVar, innerIterVar; // Original Hungarian uses prefix to denote usage (e.g. "outer iterator"), // giving the programmer an idea of the intended usage of the variable // without having to scan the code for (outerIterVar = 0UL; outerIterVar ... { for (innerIterVar = 0UL; innerIterVar ...
Having worked with MFC and WinAPI for so many years, it came as a surprise to learn this but I saw the sense in it right away & have tried to "do it properly" ever since. Unfortunately, old habits die hard... sigh.T-Mac-Oz
Larry Osterman and others have documented that Hungarian Notation originated with Charles Simonyi in the apps group at Microsoft. It was the systems group that bastardized it to the form we see in Windows where the syntactic type, which can be checked by the compiler, is encoded in the name. (Someone from the systems group, who was blamed by Larry for the error, here[^] claims that it was the people who authored the documentation that screwed it up!) I think true Hungarian would have called your variables iOuter and iInner, where the i prefix indicates an index into an array, but of course the problem is that i has been overloaded in "Systems Hungarian" (or "Anti-Hungarian" as Rick Schaut calls it[^]) to mean 'int'.
DoEvents: Generating unexpected recursion since 1991
-
Shog9 wrote:
Do people not know how to generate assembler output from their compilers anymore?
Nope. And if they did can they read it?
A human being should be able to change a diaper, plan an invasion, butcher a hog, conn a ship, design a building, write a sonnet, balance accounts, build a wall, set a bone, comfort the dying, take orders, give orders, cooperate, act alone, solve equations, analyze a new problem, pitch manure, program a computer, cook a tasty meal, fight efficiently, die gallantly. Specialization is for insects. - -Lazarus Long
Funnily enough I was looking at JIT output last night! How do I see what the JIT compiled, anyway?[^]
DoEvents: Generating unexpected recursion since 1991
-
So somewhere on the internet, I pointed out to a newbie that his code was pretty messy and that he should start by consolidating some of his variable declarations to the beginning of the program (which was consisted of only a main(…) function/method/entry point and a lot of improperly formatted code). In walks another forum member and criticizes me for my comment saying that "it's a feature of the C++ language to be able to declare variables anywhere you need it." I was obviously pissed at that comment showing his utter disregard for code neatness and readability (and likely a malicious attack on my intelligence). I'm trying to seek agreement… so was I right when I said so?
So the creationist says: Everything must have a designer. God designed everything. I say: Why is God the only exception? Why not make the "designs" (like man) exceptions and make God a creation of man?
Declaring variables at the top of a function or program is bad idea. Consider the following:
void function()
{
int i;// use 'i' for purpose 1 ... // use 'i' for purpose 2 ... // use 'i' for purpose 3 ... if(i == 14) { // crash the program }
}
At the bottom of the function, 'i' is the accidental work product of the last place that it was used. Suppose this function is 200 lines long? Is the user supposed to hand examine every single place a variable is modified in order to understand what its value is currently being used for? Instead, declare variables as close to the point that they are actually used and make sure that they go out of scope as quickly after that as possible to reduce confusion to later maintainers. Confusion is the root of all evil in s/w. Lowell used.
-
I am already using two monitors, so I am talking about REALLY long lines! :-D
mhaines@1amadeus.com wrote:
I am already using two monitors, so I am talking about REALLY long lines!
so use two 30" monitors.... come on, use it as an excuse... you know you want them!
_________________________ Asu no koto o ieba, tenjo de nezumi ga warau. Talk about things of tomorrow and the mice in the ceiling laugh. (Japanese Proverb)
-
So somewhere on the internet, I pointed out to a newbie that his code was pretty messy and that he should start by consolidating some of his variable declarations to the beginning of the program (which was consisted of only a main(…) function/method/entry point and a lot of improperly formatted code). In walks another forum member and criticizes me for my comment saying that "it's a feature of the C++ language to be able to declare variables anywhere you need it." I was obviously pissed at that comment showing his utter disregard for code neatness and readability (and likely a malicious attack on my intelligence). I'm trying to seek agreement… so was I right when I said so?
So the creationist says: Everything must have a designer. God designed everything. I say: Why is God the only exception? Why not make the "designs" (like man) exceptions and make God a creation of man?
Ri Qen-Sin wrote:
I'm trying to seek agreement… so was I right when I said so?
yes, and no. If your code loops or branches, it matters where you declare your variables. For instance declaring all your variables up front pulls a lot of space off the stack or heap, creates overhead, and if you branch or exit early, you have created unnecessary overhead. If your code branches with an if or a switch, you never want to incur the overhead associated with unused branches. on the other side, declaring a variable inside of a loop means that this variable will be allocated and deallocated often. By moving the declaration outside of the loop, your cost is significantly less performance wise. BUT! The rule above still applies. You do not have to move it to the front of the function, though you can if you don't branch or exit early.
_________________________ Asu no koto o ieba, tenjo de nezumi ga warau. Talk about things of tomorrow and the mice in the ceiling laugh. (Japanese Proverb)
-
Also, in addition to above comments, declaring variables at the top could stem from variable name re-use in C, C++: So at the top of a function you'd have: int i, j; Then you could re-use those variables in your function multiple times and the memory would only be allocated once. for (i = 0; i < blah, i++) { for (j = 0; j < blahblah; j++) do something; } for (i = 0; i < blah, i++) { for (j = 0; j < blahblah; j++) do something else; } In todays languages it would be: for (int i = 0; i < blah, i++) { for (int j = 0; j < blahblah; j++) do something; } for (int i = 0; i < blah, i++) { for (int j = 0; j < blahblah; j++) do something else; } ... but if you try that in C++ you get a duplicate definition error on i. Just trying to justify declaring variables at the top, I don't think there's a right or wrong way. Personally, I declare them as close to the point of use, as other people have mentioned, but if they're generic, reusable index variables I do them at the top. Then again, I'm old school. Compilers optimize this stuff for us now, but it's a hard habit to break. In my head, defining int j inside another loop will cause a memory allocation each time through the first loop. Ramblin' on....
- S 50 cups of coffee and you know it's on!
Steve Echols wrote:
but if you try that in C++ you get a duplicate definition error on i.
That was just MS. Other compilers don't have that problem, though as mentioned MS can fix it with an option.
_________________________ Asu no koto o ieba, tenjo de nezumi ga warau. Talk about things of tomorrow and the mice in the ceiling laugh. (Japanese Proverb)
-
Ri Qen-Sin wrote:
so was I right when I said so?
No. C required you to declare variables at the beginning of a function, because C (and C++) allocate all local variables when the stack frame is being constructed, and it simplified the compiler. C++ doesn't require this. Making me scan back through your code looking for a variable declaration isn't being neat. It's just being rude. I actually had to explain this to a new consultant last week, when he tried to argue that throwing a dozen variable declarations at the top of a rather large function increased efficiency by avoiding allocations in a loop. Good grief. Do people not know how to generate assembler output from their compilers anymore? :suss:
Shog9 wrote:
Good grief. Do people not know how to generate assembler output from their compilers anymore?
No, most do not.... :sigh: :sigh:
_________________________ Asu no koto o ieba, tenjo de nezumi ga warau. Talk about things of tomorrow and the mice in the ceiling laugh. (Japanese Proverb)
-
Ravi Bhavnani wrote:
can't imagine I'd use one that didn't do that as part of standard optimization! Code hoisting has been around for decades and is pretty mature technology.
If the object has a constructor or destructor, the C++ standard requires that it is destroyed and a new object created on every iteration of the loop. But the compiler will construct the object in the same space each time.
DoEvents: Generating unexpected recursion since 1991