1

Which of the following is best practice according to Java coding standards

public void function1(){
 boolean valid = false;
 //many lines of code
 valid = validateInputs();
 //many lines of code
}

or 

public void function1(){
 //many lines of code
 boolean valid = validateInputs();
 //many lines of code
}

Here 'valid' will not be for returning. Its scope is internal to the function only. Sometimes only in one if condition

I usually code similar to the second case. It seems my superior does not like this and modifies the code when I put it for review. Is there some specific reason that my approach is not correct?

The disadvantage I see for the first approach is that it is very difficult to refactor the method to multiple methods at a later point.

afxgx
  • 139
  • 4
  • 11

4 Answers4

5

I would go for the second approach - not much a matter of Java coding standards here, but a matter of clean and readable code. Also, you assign the value false to valid in the first case, but that's not really correct as valid shouldn't have any value at that point.

On a side note, I won't expect a method called validateInputs() to return a boolean. There's no parameter passed, and the name is not giving an hint that the method would return something. What about refactoring your code to something like boolean validInput = isValid(input)?

manub
  • 3,990
  • 2
  • 24
  • 33
  • The first version might be flagged by tools like FindBugs as "unread value" (between initialisation and call) yielding to the third style `boolean valid;`. – Joop Eggen Jun 11 '13 at 12:23
  • what i meant is any method call and any return variable, not just a validate method and a boolean return value. A validate function was the easiest to put out as an example. – afxgx Jun 11 '13 at 12:32
  • @afxgx, that's ok - I just wanted to point out that readable code is one topic we should worry a lot, as the world is already full of crap code and we don't want to make it worst. – manub Jun 11 '13 at 12:49
2

There should always be reasoning behind a decision.

The second example is better because it is good to initialize values in the declaration.

Google has a good set of standards that is applicable to many C-type languages. The example you are referring to is shown in the 'local variables' section.

Colonel Panic
  • 1,604
  • 2
  • 20
  • 31
2

I would prefer to only declare variables within the scope they are used. This avoid accidentally using it when you shouldn't and means you can see both the declaration and the usage together, instead of having to jump to the start of your code to find it.

In the C days, you had to use the first form, because the compilers were not very smart. But the second form was added as it made the code easier to understand AFAIK.

Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
2

Whichever is better is a matter of personal taste. Every place has its own standards, so you should follow it at work.

That's one more reason I think every programmer should have their own personal projects. That way you can also code in your own style at home, so you don't get your mind stuck with just one style.

Geeky Guy
  • 9,229
  • 4
  • 42
  • 62
  • It's not really a matter of personal taste. The first is more error prone, as @Peter said. – dantuch Jun 11 '13 at 12:17
  • 1
    Without knowing more context, I wouldn't call it error prone. False might be a default value for returning, should the validateInputs method never be called due to the rest of the logic. – Geeky Guy Jun 11 '13 at 12:19
  • @Renan 'valid' will not be for returning. Its scope is internal to the function only. Sometimes only in one if condition – afxgx Jun 11 '13 at 12:21
  • @afxgx You knew that beforehand, but nobody else here did. – Geeky Guy Jun 11 '13 at 12:22
  • @Renan I have added it to the question – afxgx Jun 11 '13 at 12:28