views:

220

answers:

4

Edit: Though I've accepted David's answer, Jon's answer should be considered as well.

Which method is preferred for setting the value of a read only (get only?) Property: using a backing field or using the constructor? Assume the design is for a Property and not a Field (in the future, there may be an update that requires the Property to have a setter which would preclude using a Field).

Given the following simple example, which method is preferred? If one is preferred over the other, why?

Option 1 (backing field):

class SomeObject
{
    // logic
}

class Foo
{
    private SomeObject _myObject;
    public SomeObject MyObject
    {
        get
        {
            if( _myObject == null )
            {
                _myObject = new SomeObject();
            }
            return _myObject;
        }
    }

    public Foo()
    {
        // logic
    }
}

Option 2 (constructor):

class SomeObject
{
    // logic
}

class Foo
{
    public SomeObject MyObject { get; private set; }

    public Foo()
    {
        MyObject = new SomeObject();
        // logic 
    }
}
+6  A: 

It depends on the time needed by "new SomeObject();" and the likelihood that the getter is going to be called at all.

If it's costly to create MyObject, and won't be used every time you create an instance of Foo(), option 1 is a good idea, and that's called lazy initialization. Programs like Google Chrome use it heavily to reduce startup time.

If you're going to create MyObject every time anyways, and the getter is called very often, you'll save a comparison on each access with option 2.

David
Good point about lazy initialization.
Metro Smurf
A: 

Personally, I would do the first but move the initialization to the constructor.

class Foo
{
    private SomeObject myObject;

    public SomeObject MyObject
    {
        get { return myObject; }
    }

    public Foo()
    {
        myObject = new SomeObject();
    }
}

Why? I don't personally use private set, just cause I then have to remember which properties have backing variables and which don't, which means throughout the class code some fields use lower case and some upper case. Those minor "inconsistencies" bother my OCD self, I guess.

It's just a minor style preference, though. Six of one, half a dozen of the other. I've got nothing against option #2, and do not object to the private set idiom.

John Kugelman
What exactly is the benefit of this? It always initializes myObject (losing any lazy loading), and avoids auto-props for no apparent reason.
Matthew Flaschen
I assumed the lazy-initialization aspect was not the main thrust of the question. If that were important option 2 would be a no-go.
John Kugelman
@John - you're right in that the thrust of the question wasn't specifically about lazy initialization; rather, what's the preferred method... when to use one over the other and vice versa.
Metro Smurf
+2  A: 

I've been partial to this approach lately:

class Foo
{
    public SomeObject MyObject { get; private set; }

    public Foo()
    {
        MyObject = new SomeObject();
    }
}
Matt Hamilton
I have been doing that allot lately too.
David Basarab
you could make in internal also.
eschneider
+4  A: 

In many cases I like to make types immutable. Where possible, I like to make them properly immutable, which means avoiding automatically implemented properties entirely - otherwise the type is still mutable within the same class, which feels like a bug waiting to happen.

"Proper" immutability will include making the backing field readonly, which means you have to set it in the constructor... usually initializing it from another parameter. I find it quite rare that I can lazily create an instance without any more information, as you do in your question. In other words, this is a more common pattern for me:

public class Person
{
    private readonly string name;
    public string Name { get { return name; } }

    public Person(string name)
    {
        this.name = name;
    }
}

This becomes unwieldy when you have a lot of properties - passing them all into a single constructor can get annoying. That's where you'd want to use the builder pattern, with a mutable type used to collect the initialization data, and then a constructor taking just the builder. Alternatively, the named arguments and optional parameters available in C# 4 should make this slightly easier.

To get back to your exact situation, I'd usually write:

class Foo
{
    private readonly MyObject myObject;
    public SomeObject MyObject { get { return myObject; } }

    public Foo()
    {
        myObject = new MyObject();
        // logic 
    }
}

That is, unless constructing SomeObject is particularly expensive. It's just simpler than doing it lazily in the property and potentially having to worry about threading issues.

Now, I've been assuming that immutability is a useful goal here - but you've been talking about adding a setter. I'm not sure why you think that precludes using a field, but it certainly doesn't. For instance, you could combine the lazy instantiation from your first piece of code and having a setter like this:

class Foo
{
    private SomeObject _myObject;
    public SomeObject MyObject
    {
        get
        {
            if( _myObject == null )
            {
                _myObject = new SomeObject();
            }
            return _myObject;
        }
        set
        {
            // Do you want to only replace it if
            // value is non-null? Or if _myObject is null?
            // Whatever logic you want would go here
            _myObject = value;
        }
    }

    public Foo()
    {
        // logic
    }
}

I think the major decisions should be:

  • Do you want the type to be properly immutable?
  • Do you need lazy initialization?
  • Do you need any other logic in your properties?

If the answer to all of these is no, use an automatically implemented property. If the third answer changes to yes, you can always convert the automatically implemented property into a "normal" one later.

Jon Skeet