views:

374

answers:

11

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 :-)

+1  A: 

I like the idea in that it is much less code and more elegant, but I would be very worried about the fact that it becomes hard to look at it and tell what is going on. The only way I would consider it is to have a convention for variables set using the "lazy" way, and also to comment anywhere it is used. Now there isn't going to be a compiler or anything that will enforce those rules, so still YMMV.

In the end, for me, decisions like this boil down to who is going to be looking at it and the quality of those programmers. If you can trust your fellow developers to use it right and comment well then go for it, but if not, you are better off doing it in a easily understood and followed way. /my 2cents

Ryan Guill
+2  A: 

I prefer the first code, because a) it is such a common pattern with properties that I immediately understand it, and b) the point you raised: that there is no hidden magic that you have to go look up to understand where and when the value is being obtained.

RedFilter
+1  A: 

I don't think worrying about a developer not understanding is a good argument against doing something like this...

If you think that then you couldn't do anything for the fear of someone not understanding what you did

You could write a tutorial or something in a central repository, we have here a wiki for these kind of notes

Overall, I think it's a good implementation idea (not wanting to start a debate whether lazyloading is a good idea or not)

Juan Manuel
A: 

I like your solution as it is very clever but I don't think you win much by using it. Lazy loading a private field in a public property is definitely a place where code can be duplicated. However this has always struck me as a pattern to use rather than code that needs to be refactored into a common place.

Your approach may become a concern in the future if you do any serialization. Also it is more confusing initially to understand what you are doing with the custom type.

Overall I applaud your attempt and appreciate its cleverness but would suggest that you revert to your original solution for the reasons stated above.

Andrew Hare
+4  A: 

Surely you'd at least want the LazyPropery<T> to be a value type, otherwise you've added memory and GC pressure for every "lazily-loaded" property in your system.

Also, what about multiple-threaded scenarios? Consider two threads requesting the property at the same time. Without locking, you could potentially create two instances of the underlying property. To avoid locking in the common case, you would want to do a double-checked lock.

HTH, Kent

Kent Boogaart
+1 for locking, not too hard to work into his design though
Andrew Hare
Yes, I would propably created an inhertied class to implement the same pattern, but with thread safety in the Value getter. Then the class consumer could choose whether he wants/needs thread safety or not.
driis
+1  A: 

What I do in this case is I create a Visual Studio code snippet. I think that's what you really should do.

For example, when I create ASP.NET controls, I often times have data that gets stored in the ViewState a lot, so I created a code snippet like this:

public Type Value
{
    get
    {
        if(ViewState["key"] == null)
            ViewState["key"] = someDefaultValue;
        return (Type)ViewState["key"];
    }
    set{ ViewState["key"] = value; }
}

This way, the code can be easily created with only a little work (defining the type, the key, the name, and the default value). It's reusable, but you don't have the disadvantage of a complex piece of code that other developers might not understand.

Dan Herbert
A: 

Personally, I don't think the LazyProperty class as is offers enough value to justify using it especially considering the drawbacks using it for value types has (as Kent mentioned). If you needed other functionality (like making it multithreaded), it might be justified as a ThreadSafeLazyProperty class.

Regarding the implicit property, I like the "Value" property better. It's a little more typing, but a lot more clear to me.

C. Dragon 76
+7  A: 

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.

Hallgrim
Your point is noted about the implicit operator. It is one of those things I needed opinions about.
driis
It's good to be pedantic. +1.
dalle
+5  A: 

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.

Krzysztof Koźmic
He's not asking whether lazy-evaluated fields are valid choice (he understands the distinction when it is and when it isn't). He's asking whether the solution he presents is a good design choice.
petr k.
Well he does say that he's doing a lot of lazy inits, which seems odd to me as well. Most properties should not require lazy inits imo.
Brian Rasmussen
@petr, Yes, I know what the question was. I only say that this is completely bad idea, and why. Warning not to do cut yourself, is a valid answer for question "how do I cut myself best?"
Krzysztof Koźmic
A: 

I think this is an interesting idea. First I would recommend that you hide the Lazy Property from the calling code, You don't want to leak into your domain model that it is lazy. Which your doing with the implicit operator so keep that.

I like how you can use this approach to handle and abstract away the details of locking for example. If you do that then I think there is value and merit. If you do add locking watch out for the double lock pattern it's very easy to get it wrong.

JoshBerke
A: 

You can use C# Iterators. The following post explains a sample usage and the advantages in using it.

http://hemanshubhojak.com/Home/Post?postId=3

Hemanshu Bhojak