HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Generic 'temporary instance' class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
genericclassinstancetemporary

Problem

I've been reading about WeakPointer and WeakPointer today, and it looks very useful. Rather than just using it as-is though, I decided to write a wrapper around it that covers a common usage case for me.

Code as follows:

public class Temporary where T : class
{
    Func generator = null;
    WeakReference reference = null;

    public T Value 
    { 
        get 
        { 
            return GetValue(); 
        } 
    }

    public Temporary(T value)
        : this(value, null)
    { }

    public Temporary(Func generator)
        : this(null, generator)
    { }

    public Temporary(T value, Func generator)
    {
        reference = new WeakReference(value);
        this.generator = generator;
    }

    public T GetValue()
    {
        if (reference == null)
            return null;
        T res;
        if (!reference.TryGetTarget(out res) && generator != null)
        {
            res = generator();
            reference.SetTarget(res);
        }
        return res;
    }
}


I've tested this against my use-case and it works as I expect - the garbage collector cleans out items with no active references, and they are re-instantiated at need.

I'm looking for critiques of the implementation and suggestions for improvements/additions.

Solution

It's pretty good, clean and simple code. Not a lot to critique really. A few minor things:

-
Some people (myself included) prefer to prefix private fields with _. This way it's easy to see that it's a field rather than a local variable (and gets rid of the this. in most cases).

-
You shouldn't have a public property and a public method which do the same thing. How is a programer supposed to know whether to use Value or GetValue? It's not obvious what the difference is or if there is one at all.

-
Not 100% sure about the null check in GetValue(). There is no code path where reference should ever be null. So it would actually indicate a bug if it were the case which you are hiding with this check. Consider removing the check or actually throwing an InvalidOperationException or similar instead. Detecting bugs early is important. Another option would be to look at Code Contracts to state the invariants of that method.

-
I have started to try and avoid null checks where possible by making sure that there is always an object. In your case this could be something like this:

public class Temporary where T : class
{
    private static Func NullGenerator = () => null;

    Func generator = NullGenerator;
    WeakReference reference = null;

    public T Value
    {
        get
        {
            return GetValue();
        }
    }

    public Temporary(T value)
        : this(value, NullGenerator)
    { }

    public Temporary(Func generator)
        : this(null, generator)
    { }

    public Temporary(T value, Func generator)
    {
        if (generator == null)
            throw new ArgumentNullException("generator");
        reference = new WeakReference(value);
        this.generator = generator;
    }

    private T GetValue()
    {
        T res;
        if (!reference.TryGetTarget(out res))
        {
            res = generator();
            reference.SetTarget(res);
        }
        return res;
    }
}

Code Snippets

public class Temporary<T> where T : class
{
    private static Func<T> NullGenerator = () => null;

    Func<T> generator = NullGenerator;
    WeakReference reference = null;

    public T Value
    {
        get
        {
            return GetValue();
        }
    }

    public Temporary(T value)
        : this(value, NullGenerator)
    { }

    public Temporary(Func<T> generator)
        : this(null, generator)
    { }

    public Temporary(T value, Func<T> generator)
    {
        if (generator == null)
            throw new ArgumentNullException("generator");
        reference = new WeakReference(value);
        this.generator = generator;
    }

    private T GetValue()
    {
        T res;
        if (!reference.TryGetTarget(out res))
        {
            res = generator();
            reference.SetTarget(res);
        }
        return res;
    }
}

Context

StackExchange Code Review Q#36754, answer score: 6

Revisions (0)

No revisions yet.