tags:

views:

351

answers:

8

In Java what is the best practice of initializing the private string members

class A {
       private String mem = ??//
       private int num;
       A (int a) {
          this.num = a;
       }
       A (int a, String s) {
           this.num = a;
           this.mem  = s; //What if s is null (Is this a good practice)
       }

}
+3  A: 

In short there is no best practice.

Depends on your model. Some strings may need to be nullable and others (mandatory) should always contain a value. Sometimes more than just an empty string.

I wouldn't blindly allocate the empty string to everything.

cherouvim
+3  A: 

Leave it blank: private String mem;

With regard to "What if s is null", one should make it a point to construct all objects in a valid state. But what is "valid" depends completely on your application. When the rest of your program references mem and finds it to be null, what will happen? Would it be better to have it be an empty string or some other default value? It varies depending on the circumstances.

erickson
leaving it blank is the same as initializing to null (except for final members)
Kip
That's right, Kip!
erickson
A: 

You should define a contract and then code to it. If you allow the member to be null, then initialize it to null. If not, usually initializing to empty string "" is a good idea. But again, it depends on your data model, sometimes empty string is invalid too.

In all cases, your constructor should throw NullPointerException or IllegalArgumentException if an invalid string is passed in.

Kip
+2  A: 

It depends on the usage of that attribute.

Is a null a valid input in your program?

If it's not, then you should validate it and throw a Npe.

A ( int a, String s  ) { 
    if( s == null ) { 
        throw new NullPointerException("The value of s cannot be null"); 
    }
    ....
}

If it is a valid value, then there won't be a problem.

It is a good practice though that you assign the value only in one place, so you don't get surprises if you validate in the constructor, but later a setter change the value.

OscarRyz
+1  A: 

If the String is never supposed to be null, then the constructor that sets it from a parameter should throw an Exception if the parameter is null, and the other constructor should set it to a useful default value.

Michael Borgwardt
+4  A: 

Totally depends on what you are doing...

Ask yourself these questions (whenever you make any variable)

  • will the value ever change?
  • what are the allowed valued?

In this case if "mem" and "num" will never change then mark them as final (you can always take away the final later if you find you will have to change them).

Also in this case can "mem" ever be null? Can num ever be < 0?

Here is an example of the changes I might make:

class A { private final String mem; private int num;

   A (final int a) 
   {
       this(a, null); 
   }

   A (final int a, 
      final String s) 
   {
       if(a < 0)
       {
           throw new IllegalAgumentException("a cannot be < 0, was: " + a);
       }

       num = a;
       mem = s; 
   }

}

TofuBeer
What's the purpose of making the parameter a final?
Steve Kuo
it stops you from accidentally doing a = num; or s = mem; It is also consistent with the "will the value ever change?" question. Parameters are almost never reassigned on purpose.
TofuBeer
+3  A: 

There is a trick with this (and most variables, actually).

If you NEVER initialize the variable, you are actually giving the compiler more information.

In some cases you have to, but this case is really interesting:

String s;

if(a==5)
    s="It was Five";
if(a==6)
    s="It was Six";

System.out.print(s);

This is very bad code, but the neat thing is, Java will catch it and tell you "Hey, there is a good chance that you'll hit that print with a null".

If you had started with either of these two lines:

String s=null;
String s="";

The compiler couldn't have helped you with that mistake. (It's not likely that you meant to print "", is it?)

The correct fix when you see this "variable might not have been initialized" message is NOT to initialize the string to null or "". It's to actually be sure s is set to a valid value in every possible path:

if(a==5)
    s="It was Five";
else if(a==6)
    s="It was Six";
else
    s="It was something else, who knows?";

This will compile fine, even without initializing s. This works on complex if/else, looping and exception cases, so don't just assume it will only catch trivial cases like this!

So "Best Practice" is to NEVER initialize variables to "", 0 or null. Don't initialize them unless it really makes sense and you are using the value you set it to in some way.

Once you get the error in my example, instead of the else you might just initialize s to "It was something else, who knows?" instead of adding the else--that would be perfectly valid; but by not initializing it to null or "" as a matter of habit, at least it was able to remind you that you had to do so.

Bill K
the else if version also lets you declare it as "final String s;"
TofuBeer
Very good point--I forgot to mention that. +1 tofu :)
Bill K
+1  A: 

If the String is never supposed to be null, you can declare it final, and this way the compiler makes sure that all constructors assign a value to it.

class A {
   private final String mem;
   private int num;
   A (int a) {
      this.num = a; // <= compile error, mem must be assigned
   }
   A (int a, String s) {
       this.num = a;
       this.mem  = s; // <= you need to null-check yourself
   }
}

It is still up to the constructors to make sure that they don't just assign null.

Maybe the future will bring a @NotNull annotation...

Thilo