Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Implementing a "LazyProperty" class - is this a good idea?

I often find myself writing a property that is evaluated lazily. Something like:

if (backingField == null) 
  backingField = SomeOperation();
return backingField;

It is not much code, but it does get repeated a lot if you have a lot of properties.

I am thinking about defining a class called LazyProperty:

public class LazyProperty<T>
    {
    private readonly Func<T> getter;

    public LazyProperty(Func<T> getter)
    {
        this.getter = getter;
    }

    private bool loaded = false;
    private T propertyValue;

    public T Value
    {
        get
        {
            if (!loaded)
            {
                propertyValue = getter();
                loaded = true;
            }
            return propertyValue;
        }
    }

    public static implicit operator T(LazyProperty<T> rhs)
    {
        return rhs.Value;
    }
}

This would enable me to initialize a field like this:

first = new LazyProperty<HeavyObject>(() => new HeavyObject { MyProperty = Value });

And then the body of the property could be reduced to:

public HeavyObject First { get { return first; } }

This would be used by most of the company, since it would go into a common class library shared by most of our products.

I cannot decide whether this is a good idea or not. I think the solutions has some pros, like:

  • Less code
  • Prettier code

On the downside, it would be harder to look at the code and determine exactly what happens - especially if a developer is not familiar with the LazyProperty class.

What do you think ? Is this a good idea or should I abandon it ? Also, is the implicit operator a good idea, or would you prefer to use the Value property explicitly if you should be using this class ?

Opinions and suggestions are welcomed :-)

like image 613
driis Avatar asked Jan 13 '09 18:01

driis


2 Answers

Just to be overly pedantic:

Your proposed solution to avoid repeating code:

private LazyProperty<HeavyObject> first = 
  new LazyProperty<HeavyObject>(() => new HeavyObject { MyProperty = Value });
public HeavyObject First { 
  get { 
    return first; 
  } 
}

Is actually more characters than the code that you did not want to repeat:

private HeavyObject first;
public HeavyObject First { 
  get {
    if (first == null) first = new HeavyObject { MyProperty = Value };
    return first;
  }
}

Apart from that, I think that the implicit cast made the code very hard to understand. I would not have guessed that a method that simply returns first, actually end up creating a HeavyObject. I would at least have dropped the implicit conversion and returned first.Value from the property.

like image 140
Hallgrim Avatar answered Oct 06 '22 09:10

Hallgrim


Don't do it at all.

Generally using this kind of lazy initialized properties is a valid design choice in one case: when SomeOperation(); is an expensive operation (in terms of I/O, like when it requires a DB hit, or computationally) AND when you are certain you will often NOT need to access it.

That said, by default you should go for eager initialization, and when profiler says it's your bottleneck, then change it to lazy initialization.

If you feel urge to create that kind of abstraction, it's a smell.

like image 31
Krzysztof Kozmic Avatar answered Oct 06 '22 08:10

Krzysztof Kozmic