Code Neatness
-
In addition to the other replies... Besides readability, there's also variable scope in C++. Everything at the top means method-wide scope - that's not always desired! Mark
Mark Salsbery Microsoft MVP - Visual C++ :java:
Yes, I'm aware of that: 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. I like to keep definitions in one spot so I know what variables I have.
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?
-
Matter of preference, really, as long as you don't declare inside a loop... but, yeah, I generally put them at the top if the method is not long. Otherwise, I group them in blocks near where they are used...
-
Yes, I'm aware of that: 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. I like to keep definitions in one spot so I know what variables I have.
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 like to keep definitions in one spot so I know what variables I have.
Why? Whilst much of this is 'in the eye of the beholder' and so there is no right answer, if the objective is clear and understandable code, what advantange is there in separating the declaration of a variable from its first use? Additionally, in C++, for classes with a constructor, the declaration is a function call and so the location of this call is often determined otherwise, e.g.
CString fname; // work out and assign something to fname ofstream out(fname); // do something with outI can't see how reorganising the code to declare 'out' at the beginning 'so that you know what variables you have' in any way would improve the code.Peter "Until the invention of the computer, the machine gun was the device that enabled humans to make the most mistakes in the smallest amount of time."
-
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?
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!
-
Yes, I'm aware of that: 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. I like to keep definitions in one spot so I know what variables I have.
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?
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:
-
Ravi Bhavnani wrote:
as long as you don't declare inside a loop Why not?
Because every time around the loop the stack will be adjusted for the "new" variable, and then re-adjusted as the variable goes "out of scope". Try it a million times or so and see what happens. Seriously, 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? Apropos the OP's question. You started programming in C, didn't you... Do you still use Hungarian notation? Some times it's better to just let go. ;-) You could try using more blocks if you absolutely must have declarations at the opening paren. Cheers, P.
-
Ravi Bhavnani wrote:
as long as you don't declare inside a loop Why not?
Because every time around the loop the stack will be adjusted for the "new" variable, and then re-adjusted as the variable goes "out of scope". Try it a million times or so and see what happens. Seriously, 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? Apropos the OP's question. You started programming in C, didn't you... Do you still use Hungarian notation? Some times it's better to just let go. ;-) You could try using more blocks if you absolutely must have declarations at the opening paren. Cheers, P.
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
-
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:
n 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.
I was under the impression that this is an error in your compiler. If you're using Visual Studio, go to the Project Property Pages, C/C++ -> Language -> Force Conformance in For Loop Scope, and select "Yes". That ought to fix your problem. Cheers, P.
-
Steve Echols wrote:
n 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.
I was under the impression that this is an error in your compiler. If you're using Visual Studio, go to the Project Property Pages, C/C++ -> Language -> Force Conformance in For Loop Scope, and select "Yes". That ought to fix your problem. Cheers, P.
Yeah, you're right, I was thinking in C#/VB. :)
- S 50 cups of coffee and you know it's on!
-
Ravi Bhavnani wrote:
as long as you don't declare inside a loop Why not?
Because every time around the loop the stack will be adjusted for the "new" variable, and then re-adjusted as the variable goes "out of scope". Try it a million times or so and see what happens. Seriously, 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? Apropos the OP's question. You started programming in C, didn't you... Do you still use Hungarian notation? Some times it's better to just let go. ;-) You could try using more blocks if you absolutely must have declarations at the opening paren. Cheers, P.
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
-
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:
If the code loops a million times, I think we have a much bigger problem.
Like tons of legacy data stored in a stupid system....I could go on but I'd be moved to the soapbox.
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
-
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:
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
-
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
-
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
Thanks T-Mac-Oz. I had heard something along these lines some time ago, in a post by Joel Spolsky. See if I can find it again... http://www.joelonsoftware.com/articles/Wrong.html[^] Unfortunately, as you say, the "names carries the type" version of Hungarian notation is the one that most people understand. Personally, I don't understand why people try to perservere with this (name carries type) in modern C++. I argue that C++ removes the value in doing this. Apart from the simple fact that you aren't going to attempt to put the type into variables that are instances of classes, isn't one of the original design goals of C++ to limit the scope of the code that needs access to the instances of fundamental types? Sadly enough, I usually lose these arguments. ;-) Just the other day we had a coding standards meeting for a new project. Sure enough, in an hour we didn't manage to progress beyond the standard for encoding scope and type into variable names. (I'm happy enough to prepend something for scope, btw.) But m_kpkdwName (member data const pointer to const dWord)??? Seriously? It just aggravates me. I'll go home and pop some Prozac and tomorrow it'll all be good again. Cheers, P.
-
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?
Robo George should come and answer this question! :-D
Maxwell Chen
-
Wouldn't one name those variables
dwOuteranddwInnerinstead? Having "var" in a variable's name looks quite redundant to me.joerg-schumann wrote:
Having "var" in a variable's name looks quite redundant to me.
There might be occasion to distinguish between locally defined* vars & consts, sheesh, I don't know, it was only an example! *actually, there's a good example of Hungarian gone right, the oft used "g" prefix for global vars - though not by M$ (just off the top of my head! no flames please if I've forgotten one somewhere!). The "m_" prefix is a positive example frequently used by M$ which makes it very clear that a member variable is being used, though this one seems to get bagged a lot, usually by non (or even some former) C++ programmers.
T-Mac-Oz
-
Thanks T-Mac-Oz. I had heard something along these lines some time ago, in a post by Joel Spolsky. See if I can find it again... http://www.joelonsoftware.com/articles/Wrong.html[^] Unfortunately, as you say, the "names carries the type" version of Hungarian notation is the one that most people understand. Personally, I don't understand why people try to perservere with this (name carries type) in modern C++. I argue that C++ removes the value in doing this. Apart from the simple fact that you aren't going to attempt to put the type into variables that are instances of classes, isn't one of the original design goals of C++ to limit the scope of the code that needs access to the instances of fundamental types? Sadly enough, I usually lose these arguments. ;-) Just the other day we had a coding standards meeting for a new project. Sure enough, in an hour we didn't manage to progress beyond the standard for encoding scope and type into variable names. (I'm happy enough to prepend something for scope, btw.) But m_kpkdwName (member data const pointer to const dWord)??? Seriously? It just aggravates me. I'll go home and pop some Prozac and tomorrow it'll all be good again. Cheers, P.
-
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 iAlthough the more readable way would be to declare it as:Int32 rowCount;~ Arun
