current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite

Is this good practice?

int a;
while(true) {
  a = foo();
  // a is only used in this loop not outside of it
  // some more code ...
}

Or should the variable scope be reduced to:

while(true) {
  int a = foo();
  // a is only used in this loop not outside of it
  // some more code...
}

This is obviously a style issue, but is this also a performance issue? And what about Objects instead of ints? I would like to know the way to go.

Edit: I know while(true) is horrible, but that's not what this question is about.

share|improve this question

closed as off-topic by ckuhn203, DarinDouglass, Jerry Coffin, Malachi, Marc-Andre Jun 12 at 15:10

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions must involve real code that you own or maintain. Questions seeking an explanation of someone else's code are off-topic. Pseudocode, hypothetical code, or stub code should be replaced by a concrete example." – ckuhn203, DarinDouglass, Jerry Coffin, Malachi, Marc-Andre
If this question can be reworded to fit the rules in the help center, please edit the question.
    
This is a concrete minimal working example that I made myself, isn't it? –  martijnn2008 Jun 12 at 16:48
    
But it's not real code. The a=foo is a dead give away that this is not real code. I think this question would be ok on Programmers, but I'm not sure. If you decide to post there, be sure to check to make sure it's on topic first. –  ckuhn203 Jun 12 at 18:00
add comment

2 Answers

up vote 2 down vote accepted

It depends on the language you're writing in.

To quote Douglas Crockford:

In languages with block scope, it is usually recommended that variables be declared at the site of first use. But because JavaScript does not have block scope, it is wiser to declare all of a function's variables at the top of the function.

So if this is JavaScript, and you trust Douglas Crockford's judgment, which a great many people do, the first snippet is the way to go. However, in a language with non-screwy lexical scoping, which is the majority of mainstream languages today, the second snippet is recommended.

share|improve this answer
add comment
up vote 4 down vote

Not just in this case, but in general, you want to give any particular variable the minimum scope necessary to do its job.1

There are, however, a few rare cases where it can make sense to give a variable a larger scope than strictly necessary. The typical reason (excuse?) is performance--if the variable is of a type that's extremely expensive to create and/or destroy, but very cheap to reuse and performance of that part of the code is a real bottleneck, it can sometimes be worth moving it out to a larger scope where it can (for example) be created only once before the loop starts and destroyed once after the loop ends, instead of having to create and destroy it every iteration through the loop.

In such a case, however, it's typically better to have a wrapper that acts a little like a singleton. It creates a single instance of the underlying (expensive) object when needed and uses that as long as needed. Then when you're completely done with it, that underlying instance is destroyed. I've only run into this a few times though, and the circumstances have varied enough that I can't give much more than rather hand-waving generalities about it as a general case.

Other than that, I'd generally prefer to avoid the while (true) as well, except in the rather rare case that you really want a loop that never exits. In most cases you end up with a if (x) break; (or something similar) somewhere inside the loop. In most cases you can move that loop exit criteria into the loop header where it belongs, so somebody reading the code can see the exit criteria for the loop without having to scan through its entire body looking for it.

With some care, this can also give some guidance about the intent of the loop, so when somebody's looking for a specific part of the code (i.e., most typical code reading) they can know to skip ahead across the entirety of the loop if it's unrelated to what they care about right now. With a while (true), they're pretty much stuck with reading through the body of the loop to divine its intent, thus typically wasting quite a bit of time on irrelevant code.

1. Yes, there are a few languages that make this choice problematic. As Lincoln points out in his answer, JavaScript is one of them. The answer, IMO, is not to write crappy code to suit those languages. The answer is to expunge such horrible languages from the face of the earth, and get on with life using languages that aren't such a mess.

share|improve this answer
    
the while(true) is just to have a loop :) I did't asked for a code review, so you can leave the discussion about while(true) out of your answer. –  martijnn2008 Jun 12 at 15:01
1  
@martijnn2008: You seem to misunderstand the intent of this site. It's a code review site. If you just want to ask a specific question about one aspect of hypothetical code, that belongs on Stack Overflow. Here, you post code for review, and reviewers can generally be expected to look at all aspects of what you post. –  Jerry Coffin Jun 12 at 15:04
    
Uhmm, ok fair enough. But I though codereview is also about code style and best practices. –  martijnn2008 Jun 12 at 15:06
2  
@martijnn2008: yes, and while (true) is poor style/practice in most cases, therefore worthy of comment here. –  Jerry Coffin Jun 12 at 15:07
1  
@LincolnBergeson: I can't take (even close to) sole credit, but I've lived through FORTRAN, line-number oriented BASIC, dBASE, etc. JavaScript too shall pass... –  Jerry Coffin Jun 12 at 16:56
show 1 more comment

Not the answer you're looking for? Browse other questions tagged or ask your own question.