views:

169

answers:

3

Suppose I have created a wrapper class like the following:

public class Foo : IFoo
{
    private readonly IFoo innerFoo;

    public Foo(IFoo innerFoo)
    {
        this.innerFoo = innerFoo;
    }

    public int? Bar { get; set; }
    public int? Baz { get; set; }
}

The idea here is that the innerFoo might wrap data-access methods or something similarly expensive, and I only want its GetBar and GetBaz methods to be invoked once. So I want to create another wrapper around it, which will save the values obtained on the first run.

It's simple enough to do this, of course:

int IFoo.GetBar()
{
    if ((Bar == null) && (innerFoo != null))
        Bar = innerFoo.GetBar();
    return Bar ?? 0;
}

int IFoo.GetBaz()
{
    if ((Baz == null) && (innerFoo != null))
        Baz = innerFoo.GetBaz();
    return Baz ?? 0;
}

But it gets pretty repetitive if I'm doing this with 10 different properties and 30 different wrappers. So I figured, hey, let's make this generic:

T LazyLoad<T>(ref T prop, Func<IFoo, T> loader)
{
    if ((prop == null) && (innerFoo != null))
        prop = loader(innerFoo);
    return prop;
}

Which almost gets me where I want, but not quite, because you can't ref an auto-property (or any property at all). In other words, I can't write this:

int IFoo.GetBar()
{
    return LazyLoad(ref Bar, f => f.GetBar());  // <--- Won't compile
}

Instead, I'd have to change Bar to have an explicit backing field and write explicit getters and setters. Which is fine, except for the fact that I end up writing even more redundant code than I was writing in the first place.

Then I considered the possibility of using expression trees:

T LazyLoad<T>(Expression<Func<T>> propExpr, Func<IFoo, T> loader)
{
    var memberExpression = propExpr.Body as MemberExpression;
    if (memberExpression != null)
    {
        // Use Reflection to inspect/set the property
    }
}

This plays nice with refactoring - it'll work great if I do this:

return LazyLoad(f => f.Bar, f => f.GetBar());

But it's not actually safe, because someone less clever (i.e. myself in 3 days from now when I inevitably forget how this is implemented internally) could decide to write this instead:

return LazyLoad(f => 3, f => f.GetBar());

Which is either going to crash or result in unexpected/undefined behaviour, depending on how defensively I write the LazyLoad method. So I don't really like this approach either, because it leads to the possibility of runtime errors which would have been prevented in the first attempt. It also relies on Reflection, which feels a little dirty here, even though this code is admittedly not performance-sensitive.

Now I could also decide to go all-out and use DynamicProxy to do method interception and not have to write any code, and in fact I already do this in some applications. But this code is residing in a core library which many other assemblies depend on, and it seems horribly wrong to be introducing this kind of complexity at such a low level. Separating the interceptor-based implementation from the IFoo interface by putting it into its own assembly doesn't really help; the fact is that this very class is still going to be used all over the place, must be used, so this isn't one of those problems that could be trivially solved with a little DI magic.

The last option I've already thought of would be to have a method like:

 T LazyLoad<T>(Func<T> getter, Action<T> setter, Func<IFoo, T> loader) { ... }

This option is very "meh" as well - it avoids Reflection but is still error-prone, and it doesn't really reduce the repetition that much. It's almost as bad as having to write explicit getters and setters for each property.

Maybe I'm just being incredibly nit-picky, but this application is still in its early stages, and it's going to grow substantially over time, and I really want to keep the code squeaky-clean.

Bottom line: I'm at an impasse, looking for other ideas.

Question:

Is there any way to clean up the lazy-loading code at the top, such that the implementation will:

  • Guarantee compile-time safety, like the ref version;
  • Actually reduce the amount of code repetition, like the Expression version; and
  • Not take on any significant additional dependencies?

In other words, is there a way to do this just using regular C# language features and possibly a few small helper classes? Or am I just going to have to accept that there's a trade-off here and strike one of the above requirements from the list?

+3  A: 

If you can use .NET 4, you should just use Lazy<T>.

It provides the functionality you are after, and, in addition, is fully thread-safe.

If you are not able to use .NET 4, I'd still recommend looking at it, and "stealing" it's design and API. It makes lazy instantiation quite easy.

Reed Copsey
I did look into that class (still on .NET 3.5, but I could implement that myself) - but wasn't really able to work out how I would fit it into the design, keeping in mind that `innerFoo` might actually be `null` and that external classes should be able to change the property values (hence the public setters). Maybe I'm just not seeing the forest for the trees - could you perhaps include an example of how this would look using `Lazy<T>`?
Aaronaught
+1  A: 

If all you're trying to do is avoid replicating this code 300 times:

private int? bar;
public int Bar
{
    get
    {
        if (bar == null && innerFoo != null)
           bar = innerFoo.GetBar();
        return bar ?? 0;           
    }
    set
    {
        bar = value;
    }
}

Then you could always just create an indexer.

enum FooProperties
{
    Bar,
    Baz,
}

object[] properties = new object[2];

public object this[FooProperties property]
{
    get
    {
        if (properties[property] == null)
        {
           properties[property] = GetProperty(property);
        }
        return properties[property];           
    }
    set
    {
        properties[property] = value;
    }
}

private object GetProperty(FooProperties property)
{
    switch (property)
    {
         case FooProperties.Bar:
              if (innerFoo != null)
                  return innerFoo.GetBar();
              else
                  return (int)0;

         case FooProperties.Baz:
              if (innerFoo != null)
                  return innerFoo.GetBaz();
              else
                  return (int)0;

         default:
              throw new ArgumentOutOfRangeException();
    }
}

This will require casting the value when it's read:

int myBar = (int)myFoo[FooProperties.Bar];

But it avoids most of the other problems.

EDITED TO ADD:

OK, here's what you should do, but don't tell anybody that you did it, or that I suggested it. Go with this option:

int IFoo.GetBar()
{
    return LazyLoad(ref Bar, f => f.GetBar());  // <--- Won't compile
}

Making Bar, Baz, and friends public fields instead of properties. That should be exactly what you're looking for.

But, again, don't tell anybody that you did that!

Jeffrey L Whitledge
Thanks, I appreciate the response. I'm not sure that I'd want to go with this, as the indexer and enumeration would need to be custom-written for every lazy class, and the null check is still being repeated several times (just in a different place). It's sort of grouping all the ugliness together, which is an improvement, but I want to eliminate it entirely. ;)
Aaronaught
@Aaronaught - I added another (secret) option to my answer.
Jeffrey L Whitledge
+1  A: 

I ended up implementing something that's kinda sorta similar to the Lazy class in .NET 4, but more customized to the specific notion of "caching" as opposed to "lazy loading".

The quasi-lazy class looks like this:

public class CachedValue<T>
{
    private Func<T> initializer;
    private bool isValueCreated;
    private T value;

    public CachedValue(Func<T> initializer)
    {
        if (initializer == null)
            throw new ArgumentNullException("initializer");
        this.initializer = initializer;
    }

    public CachedValue(T value)
    {
        this.value = value;
        this.isValueCreated = true;
    }

    public static implicit operator T(CachedValue<T> lazy)
    {
        return (lazy != null) ? lazy.Value : default(T);
    }

    public static implicit operator CachedValue<T>(T value)
    {
        return new CachedValue<T>(value);
    }

    public bool IsValueCreated
    {
        get { return isValueCreated; }
    }

    public T Value
    {
        get
        {
            if (!isValueCreated)
            {
                value = initializer();
                isValueCreated = true;
            }
            return value;
        }
    }
}

The idea is, unlike the Lazy<T> class, this can also be initialized from a specific value. I also implemented some implicit conversion operators, so that it's possible to directly assign values to CachedValue<T> properties as though they were just T. I didn't implement the thread-safety features of Lazy<T> - these instances are not designed to be passed around.

Then, since it's very verbose to instantiate these things, I took advantage of some generic type inference features to create a more compact lazy-init syntax:

public static class Deferred
{
    public static CachedValue<T> From<TSource, T>(TSource source, 
        Func<TSource, T> selector)
    {
        Func<T> initializer = () =>
            (source != null) ? selector(source) : default(T);
        return new CachedValue<T>(initializer);
    }
}

At the end of the day, what this gets me is an almost-POCO class using auto-properties that are initialized with deferred loaders in the constructor (which are null-coalescing from Deferred):

public class CachedFoo : IFoo
{
    public CachedFoo(IFoo innerFoo)
    {
        Bar = Deferred.From(innerFoo, f => f.GetBar());
        Baz = Deferred.From(innerFoo, f => f.GetBaz());
    }

    int IFoo.GetBar()
    {
        return Bar;
    }

    int IFoo.GetBaz()
    {
        return Baz;
    }

    public CachedValue<int> Bar { get; set; }
    public CachedValue<int> Baz { get; set; }
}

I'm not absolutely thrilled with this, but I am pretty happy with it. What's good is that it also lets outsiders populate the properties in an implementation-agnostic way, which is very useful when I want to override the lazy-loading behaviour, i.e. to preload a bunch of records from a big SQL query:

CachedFoo foo = new CachedFoo(myFoo);
foo.Bar = 42;
foo.Baz = 86;

I'm sticking with this for now. It seems pretty hard to screw up a wrapper class with this approach. Because of the implicit conversion operator being used, it's even safe against null instances.

It still has a somewhat hackish feel, and I'm still open to better ideas.

Aaronaught