views:

139

answers:

5

This question is a result of the answers to this question that I just asked.

It was claimed that this code is "ugly" because it initializes a variable to a value that will never be read:

String tempName = null;
try{
    tempName = buildFileName();
}
catch(Exception e){
    ...
    System.exit(1);
}
FILE_NAME = tempName;

Is this indeed bad practice? Should one avoid initializing variables to dummy values that will never actually be used?

(EDIT - And what about initializing a String variable to "" before a loop that will concatenate values to the String...? Or is this in a separate category?

e.g.

String whatever = "";
for(String str : someCollection){
   whatever += str;
}

)

A: 

As a practice, I tend to avoid setting variables to arbitrary values, and instead initialize them to a default value.

i.e.

int x = 0;
string name = "";
bool done = false;
Person randomGuy = null; //or = new Person();

I like this method best because it brings a sense of uniformity to your code while not forcing the next guy that comes along to deal with stuff like: string name = "luke skywalker";

This is all more of a personal preference, so it will vary between programmers.

At the very lest, you should be following the standards that your project as set. You'll have an idea of how the legacy code handles these things, and it's probably best to conform to that so the overall coding style is the same throughout the system.

Robert Greiner
+1 definitely follow the standards set for the project.
Robb
A: 

It depends on the compiler. C# compiler requires that the variable be initialized before using. But the CLR does not have this requirement. At run time the CLR verifies if the variable is initialized or no. If not it will throw a nullreference exception.

Yogendra
`s/compiler/language/`
Tom Hawtin - tackline
+6  A: 

I think there's no point in initializing variables to values that won't be used unless required by the language.

For example, in C#, the default value for a declared string variable is null, so you're not even gaining anything by explicitly writing it out. It's mostly a style choice, but since strings are immutable, initializing it to something else would actually allocate an extra string in memory that you'd just throw away anyway. Other languages may impose other considerations.

Anna Lear
Your C# example is only true if you're talking about *member variables*. A local variable does not have a default value and must be initialized before it can be read (though, as Yogendra wrote, this is a C# requirement, not a CLR requirement). The compiler will complain about this, though, so I think this matches your "unless required by the language" restriction. Anyway, couldn't agree more with the actual point you're making, so it's a good +1.
OregonGhost
+1  A: 

Regarding the string loop, if you change it to a StringBuilder instead you don't even have to think about it.

Edit: removed bits better answered by others.

ho1
A: 

In my opinion, it might be more accurate to refer to it as a "code smell" - in the Martin Fowler sense of the word.

I don't think you can change your default initialisation in isolation - it would need to be made in conjunction with other refactoring methods. It is also assuming that you have refactored your code so that you don't need temp variables:

try{  
    FILE_NAME = buildFileName();  
    //Do other stuff with file name
}  
catch(Exception e){  
    ...  
    System.exit(1);  
} 

It also then makes the assumtion that this code segment is only code in the method in which it is contained - i.e. the method only does one thing

When I'm coding I would be concerned that I am using dummy values with temp variables but I would only change when I am finished coding that section and it solves the problem as intended - and only in conjunction with other refactoring steps.

David Relihan