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?
I've been perpetually seeking the "cleanest" way to code. Here are my thoughts: In C, one doesn't need to consider subtle features like constructor/destructor behavior, operator overloading, inheritance, generic programming, C++ exceptions, and the like. In C++, these possibilities may associate extra functionality (e.g resource aquisition is initialization, RAII) with the declaration of an object, and also with the departure from the object's scope. Say you declare an object variable inside a function or method whose usage is sometimes skipped over. Say also that the object's constructors are designed to acquire some system resource (e.g. a lock, not uncommon in multi-threading ). If you've declared the object at the top of your routine, you'd be unnecessarily acquiring that resource (e.g if the shared data the lock protects isn't referenced ). This can manifest as unnecessary delay or resource starvation. This justifies declaring some object-types close to their usage. As for basic types: since it's already justified for some objects, you may as well do it with basic types to keep standards consistent and code modular. This also makes it easier to break a big routine into sub-routines. Not to mention, you never know if a future code refactor may turn a basic type into an object. As I've experienced, you may need to change your doubles into special fixed-point objects to run better on particular machines ... like the UltraSPARC T1000 ... C++ is subtle. IMO, the compiler ought to be explicit about how much of this it automatically handles. See: http://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization[^] - Charles Rojo Software Engineer
Rojo
modified on Monday, March 10, 2008 11:42 PM
-
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)
El Corazon wrote:
For instance declaring all your variables up front pulls a lot of space off the stack
false. modern compilers all calculate the max stack need for all variables declared within a method's scope and allocate the stack space. The stack does *not* grow and shrink during a method's lifetime (even if that method has embedded sub-scopes), except as a result of other methods called from within that method.
El Corazon wrote:
declaring a variable inside of a loop means that this variable will be allocated and deallocated often
this is only true for non-primitive variables.
Silence is the voice of complicity. Strange women lying in ponds distributing swords is no basis for a system of government. -- monty python Might I suggest that the universe was always the size of the cosmos. It is just that at one point the cosmos was the size of a marble. -- Colin Angus Mackay
-
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?
Hi, Here's my two cents ;) I used to think this was just a personal choice (unless company coding standards said so), until I tried debugging code with the (class-level) declares scattered all over the document... it made me dizzy watching the code window leap all over the place, and it seemed slower. I've gone back to declaring everything at the top. In fact, in C# I even have a region I place all my class level declarations in. In specific methods, I generally only declare variables at the top of the method if I'm going to use them in side various branch statements, if they're only used inside a single branch then I declare them there. The comment about declaring them late to prevent unneccesary construction, would at least in some languages, only apply to variables that were set to a new value during construction, rather than simply declared, and is more about performance of the code rather than readability or maintainability. As for the comments about long lines of code... I used to care about this too, until I realised some of our other company developers still have a single monitor at 1024x768... at which point, in C# with various docked windows, 15 characters is too long. Basically, you never know how big someone's text window is, so I just write what I consider 'reasonable' for the paritcular line, and if someone complains, they can reformat it. Oh, but my code lines are usually shorter than my sentences :)
-
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?
Nope, you were wrong as far as standard best practice goes (see Code Complete as an authoritative example). While in C this is the standard practice, in C++ it is considered the standard practice to code your var declarations/definitions at the point of first use. If you think about it, this makes sense for an OOP language. The idea is that in C++, you want good encapsulation and easier maintainability. Problem is that few who claim they are coding in C++ are doing little more than compiling C with a C++ compiler. However, the bottom line is that as the programmer, you get to do it any way you find best. Probably a better way of "teaching" is just to post an example of how you would do it, and let the other person take what they can from it. Patrick
-
Choosing The Best Overload Operator : In C++, overload +,-,*,/ to do things totally unrelated to addition, subtraction etc. After all, if the Stroustroup can use the shift operator to do I/O, why should you not be equally creative? If you overload +, make sure you do it in a way that i = i + 5; has a totally different meaning from i += 5; Here is an example of elevating overloading operator obfuscation to a high art. Overload the '!' operator for a class, but have the overload have nothing to do with inverting or negating. Make it return an integer. Then, in order to get a logical value for it, you must use '! !'. However, this inverts the logic, so [drum roll] you must use '! ! !'. Don't confuse ! operator, which returns a boolean 0 or 1, with the ~ bitwise logical negation operator.I think I'm missing something here, how exactly is the ! operator supposed to be implemented to get the desired result from !!!?Otherwise [Microsoft is] toast in the long term no matter how much money they've got. They would be already if the Linux community didn't have it's head so firmly up it's own command line buffer that it looks like taking 15 years to find the desktop. -- Matthew Faithfull
dan neely wrote:
I think I'm missing something here, how exactly is the ! operator supposed to be implemented to get the desired result from !!!?
Thus proving it's effectiveness... (i.e. I don't know either - haven't had a chance to read the whole document unfortunately :^) )
"Silently laughing at silly people is much more satisfying in the long run than rolling around with them in a dusty street, trying to knock out all their teeth. If nothing else, it's better on the clothes." - Belgarath (David Eddings)
-
Shog9 wrote:
Do people not know how to generate assembler output from their compilers anymore?
Perhaps I need to look this up :P
Shog9 wrote:
Making me scan back through your code looking for a variable declaration isn't being neat. It's just being rude.
I think the degree of neat/rudeness depends on the coder's personal taste. I, personally, prefer (in most cases) variables declared/initialized at the beginning of their scope. This way, I can see all the information about all the variables in the scope. I find it much easier to debug this way, since I can immediately rule out the declaration and initialization of my variables as the cause of a problem, and seems to help me prevent conditional initializations of variables like:
int x; if (sloppy_joes == good) x = 47;If the 2 lines above are sandwiched between 50 lines of code on either side, I've often forgotten in the past that x may not always get initialized, and will think "x is declared here... and is initialized to 47 there. What the heck is wrong?!" If initialization/declaration are placed at the beginning, I think more like: "x is declared here, initialized on the next line, and used down there." I can't help but agree that code is just more readable with variable declarations (in most cases) placed at the top of the variable's scope - but that's just my take on the subject."Silently laughing at silly people is much more satisfying in the long run than rolling around with them in a dusty street, trying to knock out all their teeth. If nothing else, it's better on the clothes." - Belgarath (David Eddings)
See? You actually understand me! :)
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?
-
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
Exactly!!! That is what I meant. Mayme my choice of diction is pretty bad since everyone seems to assume that I am suggesting that all variables (no matter the scope) should be declared globally or something ridiculous like that. ~_~
-
Hi, Here's my two cents ;) I used to think this was just a personal choice (unless company coding standards said so), until I tried debugging code with the (class-level) declares scattered all over the document... it made me dizzy watching the code window leap all over the place, and it seemed slower. I've gone back to declaring everything at the top. In fact, in C# I even have a region I place all my class level declarations in. In specific methods, I generally only declare variables at the top of the method if I'm going to use them in side various branch statements, if they're only used inside a single branch then I declare them there. The comment about declaring them late to prevent unneccesary construction, would at least in some languages, only apply to variables that were set to a new value during construction, rather than simply declared, and is more about performance of the code rather than readability or maintainability. As for the comments about long lines of code... I used to care about this too, until I realised some of our other company developers still have a single monitor at 1024x768... at which point, in C# with various docked windows, 15 characters is too long. Basically, you never know how big someone's text window is, so I just write what I consider 'reasonable' for the paritcular line, and if someone complains, they can reformat it. Oh, but my code lines are usually shorter than my sentences :)
I don't know who 1'ed you for that, but you got my 5 cents. :)
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.
Can I suggest, humbly, that; 1. Having a function 200 lines long is not neat code anyway. 2. Re-using a variable for multiple purposes inside the same function is bad for neatness anyway... you should name your variables with meaningful names and only use them for their intended purpose. Perhaps you might occasionally create a temporary primitive type to perform some work on before assining it to a well named var, but then it should be named temp and no one should rely on it's value outside of the 4-5 lines that perform the initial operations... any code in the same block that wants to re-use it should reset it to a default value intead of relying on whatever it was last set to.
-
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?
I'm surprised in a talk about code neatness short methods don't come into the mix. I personally have a metric of no method longer than a screen. It's easier to give it "Functional" coherence if its short, which makes it easier to understand for me. Usually a method with two blocks does not have functional coherence, so I split it into two methods. The book "Code Complete" argued that long methods didn't matter as far as bugs were concerned but it sure makes it harder for readability for me. I always give each variable as little scope as possible, it's easier to read for me and avoids variables accidentally retaining state between multiple blocks (which i avoid see above) and then let the compiler optimize the variable allocation. All in all I have a "break it down to its smallest blocks" mentality
-
Hi, Here's my two cents ;) I used to think this was just a personal choice (unless company coding standards said so), until I tried debugging code with the (class-level) declares scattered all over the document... it made me dizzy watching the code window leap all over the place, and it seemed slower. I've gone back to declaring everything at the top. In fact, in C# I even have a region I place all my class level declarations in. In specific methods, I generally only declare variables at the top of the method if I'm going to use them in side various branch statements, if they're only used inside a single branch then I declare them there. The comment about declaring them late to prevent unneccesary construction, would at least in some languages, only apply to variables that were set to a new value during construction, rather than simply declared, and is more about performance of the code rather than readability or maintainability. As for the comments about long lines of code... I used to care about this too, until I realised some of our other company developers still have a single monitor at 1024x768... at which point, in C# with various docked windows, 15 characters is too long. Basically, you never know how big someone's text window is, so I just write what I consider 'reasonable' for the paritcular line, and if someone complains, they can reformat it. Oh, but my code lines are usually shorter than my sentences :)
-
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!
> but if you try that in C++ you get a duplicate definition error on i. Not in any standard compiler. I would argue that doing int i; for (i = ...) for (i = ... for (i = ... is bad practice because it leads the reader to think that different variables are actually related. I don't like declaring variables at the top because it leads to this kind of misleading reuse. All variables should have the smallest possible scope. Each variable instance should be used for one and only one purpose. ++PLS
-
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:
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.
Another good reason for declaring close to use is that it makes it easier to refactor a large function into smaller ones. Then you can (if you want) move your variable declarations back to the top. :)
Kevin
-
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.
I've never had that problem, and I don't see why anyone would...
// "i" does not exist at this line for (int i=0; blah; blah) { do something } // "i" exists // "i" does not exist for (int i=0; blah2; blah2) { do something else } // "i" exists // "i" does not existIn the above code, "i" in each for() loop is in a different scope - i will be taken off the stack right after the last iteration of the first for() loop, and will be recreated on the stack for the second for() loop - only to be removed from the stack again when the second for() loop is done its last iteration. You should try it. Try running this code: *edit - converted angle brackets to &xx;'s*#include <iostream> using namespace std; int main () { for (int i=0; i<10; i++) cout << "counting...\n"; cout<< "i = " << i << endl; return 0; }I can assure you that you'll get a compile error saying that "i" is out of scope or obsolete or something like that... if you don't you should get a newer compiler ;)"Silently laughing at silly people is much more satisfying in the long run than rolling around with them in a dusty street, trying to knock out all their teeth. If nothing else, it's better on the clothes." - Belgarath (David Eddings)
-
Steve Echols wrote:
but if you try that in C++ you get a duplicate definition error on i.
I've never had that problem, and I don't see why anyone would...
// "i" does not exist at this line for (int i=0; blah; blah) { do something } // "i" exists // "i" does not exist for (int i=0; blah2; blah2) { do something else } // "i" exists // "i" does not existIn the above code, "i" in each for() loop is in a different scope - i will be taken off the stack right after the last iteration of the first for() loop, and will be recreated on the stack for the second for() loop - only to be removed from the stack again when the second for() loop is done its last iteration. You should try it. Try running this code: *edit - converted angle brackets to &xx;'s*#include <iostream> using namespace std; int main () { for (int i=0; i<10; i++) cout << "counting...\n"; cout<< "i = " << i << endl; return 0; }I can assure you that you'll get a compile error saying that "i" is out of scope or obsolete or something like that... if you don't you should get a newer compiler ;)"Silently laughing at silly people is much more satisfying in the long run than rolling around with them in a dusty street, trying to knock out all their teeth. If nothing else, it's better on the clothes." - Belgarath (David Eddings)
Maybe it's just VC6, haven't tried it in VS2k3 or 2k5. Someone above mentioned that there's an option you can set to scope it properly.
- S 50 cups of coffee and you know it's on!
-
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.
If I remember correctly, "Code Complete" recommends declaring variables as close to their usage (better locality). But it also recommends using variables for just one purpose... So if you're factoring out common stuff and have little functions/methods that do only one thing, there is good locality regardless of using a consolidated style of variable declarations or not.
