Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

When setting initial values for class members we have the option of setting them in the declaration

public class MyClass {
    public int someInt = 300;
    // etc. etc.
}

or within the constructor

public class MyClass {
    public int someInt;

    public MyClass() {
        someInt = 300;
    }
}

Which of these is considered better practice? Are there any performance differences between the two?

share

locked by Jamal Nov 26 '14 at 9:23

This question exists because it has historical significance, but it is not considered a good, on-topic question for this site, so please do not use it as evidence that you can ask similar questions here. This question and its answers are frozen and cannot be changed. More info: help center.

    
Use dependency injection to inject your default value. – T. Fabre Mar 2 '12 at 13:08
up vote 11 down vote accepted

You could assign primitives in the class declaration.

But, if you're assigning classes to members you should do in the constructor.
Think about a situation where you're trying to assign some class, but its constructor fails throws an exception:

public class MyClass {
    public SomeOtherClass member1 = new SomeOtherClass();
}

The exception will be thrown in the class creation, and if you have a lot of members it gets hard to debug (You can't place a breakpoint if it's not in the constructor)

But if you assign them in the constructor you can manage the exceptions, catch them, or at least put a breakpoint.

share
    
A straightforward guideline and practical reasoning behind it. Great answer :) – Mike C Mar 2 '12 at 14:23
    
This makes perfect sense, though I would add a modifier for readability: Only do one or the other. As soon as your class requires a constructor at all, put all this initialization in the constructor. When you're reading code and trying to see what happens first, if you see a constructor you will look at that and likely ignore the member fields. So in short: Keep all your initialization code together, whether in constructor or not. – Jimmy Hoffa Mar 3 '12 at 6:25
    
Good explanation, it is really easier to debug. – Guilherme J Santos Mar 7 '12 at 14:13

I believe Microsoft's guidelines for static members of a class are to initialize members in-line rather than the static constructor. Not sure if that extends to non-static members. I'll be honest, my preference is the former. Constructors "do things" while initial values are kind of "window dressing" to the action of the class, as it were.

share

I think it depends. There is a reason why both ways are possible.

What I would strive for is to have relevant code together. So, if one field has more complicated initialization, which means it has to be set in constructor, then all related fields should be set in the constructor too.

But if the initialization is simple, it's probably better to have the fields and their default values together.

Also, don't forget that if those are not just default values, and you won't need to change them later, you should use the readonly modifier.

share

In this case you should probably use a const:

public class MyClass 
{
    private const int SomeInt = 300;
}
share
1  
-1: OP says setting initial values, which clearly implies that they can change later. – ANeves Mar 2 '12 at 14:12
    
bad friday mood? – Mattias Mar 2 '12 at 14:34
    
Quite so... was I brute? Apologies. :( I think it is a very good suggestion, but also that it does not match the question asked and so "is not helpful". – ANeves Mar 2 '12 at 18:01

Set default values in the Constructor due to Order

public class MyClass {
    public int CopyOfSomeInt = someInt; //Value of zero
    public int someInt = 300;
    // etc. etc.
}

or within the constructor

public class MyClass {
    public int someInt;

    public MyClass() {
        CopyOfSomeInt = someInt;
        someInt = 300;
        CopyOfSomeInt = someInt; //Value of 300
    }
}
share
    
-1: I don't understand what you mean. – ANeves Mar 2 '12 at 14:10
    
If you set default values on the line you declare the variable the values are set in the order they are found in the code file. If you move some variables around you can end up in a situation where the values are not set correctly as given in my example. – Barfieldmv Mar 2 '12 at 14:54
    
Oh, I see it now. I find it a very good tip, actually. Perhaps you could make it clearer by improving the text? – ANeves Mar 2 '12 at 18:04

I would set them in the constructor, since I would also recommend the use of properties over fields. The differences are subtle, but they do come up.

I would also capitalize the property name, or the public field if you left it like that:

public class MyClass
{
    public int SomeInt { get; set; }

    public MyClass()
    {
        SomeInt = 300;
    }
}
share
    
I'm curious: why would you opt for a property? If you don't need to anything special upon getting/setting then there's no reason and from what I understand properties take longer to access. (It's a miniscule difference but can make a large difference on bigger projects) – Mike C Mar 8 '12 at 22:06
    
I want to say that properties can be data bound and model bound, while public fields cannot. Sources: self, model binding, data binding, general argument – John Bubriski Mar 12 '12 at 18:05

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