CODEWITHSUNDEEP
c#
rexcfnghk
rexcfnghk

Reputation: 15452

Class design for read-only collection properties

I have an IDictionary<string, MyEnum?> collection that needs to be passed to a class to wrap it in a IReadOnlyDictionary<string, MyEnum> (note MyEnum but not MyEnum?).

I have come up with two designs:

  1. Delay the wrapping to IReadOnlyDictionary<string, MyEnum> until property access:

    public class MyClass
{
    private readonly IEnumerable<KeyValuePair<string, MyEnum?>> _kvps;

    public MyClass(IEnumerable<KeyValuePair<string, MyEnum?>> kvps)
    {
        _kvps = kvps;
    }

    public IReadOnlyDictionary<string, MyEnum> Kvps
    {
        get
        {
            var filtered = from kvp in _kvps
                           where kvp.Value.HasValue
                           select kvp;
            return new ReadOnlyDictionary<string, MyEnum>(
               filtered.ToDictionary(kvp => kvp.Key, kvp => (MyEnum)kvp.Value);
        }
    }
}

  2. Eagerly evaluate the collection in constructor

    public class MyClass
{
    public MyClass(IEnumerable<KeyValuePair<string, MyEnum?>> kvps)
    {
        Kvps = ToReadOnly(kvps);
    }

    public IReadOnlyDictionary<string, MyEnum> Kvps { get; }

    private static IReadOnlyDictionary<string, MyEnum> ToReadOnly(
        IEnumerable<KeyValuePair<string, MyEnum?>> kvps)
    {
        var filtered = from kvp in kvps
                       where kvp.Value.HasValue
                       select kvp;
        return new ReadOnlyDictionary<string, MyEnum>(
           filtered.ToDictionary(kvp => kvp.Key, kvp => (MyEnum)kvp.Value);
    }
}

The constructor design section of the Framework Design Guidelines suggests that minimal work should be done in constructors so I am opting for the first approach. However, that means every call to MyClass.Kvps will trigger a copy of _kvps which is not ideal.

I would like to know which is a better approach (or are there other ways) in terms of:

Upvotes: 1

Views: 92

Answers (2)

Scott Hannen
Scott Hannen

Reputation: 29222

Out of the two requirements - don't copy the key value pairs and don't store two copies - you'll have to break one.

What causes us to look at this and think that there must be a solution is that we see TValue and TValue? and our minds want to see them as being of the same type. But they are not the same type.

It becomes clearer if you imagine that instead of TValue and TValue? that these are two different types, like an int and a string, and we want to project a collection of one to a collection of the other while filtering. For example,

List<string> GetStringsFromNonNegativeInts(List<int> ints)
{
    return ints.Where(i=>i>-1).Select(i=>i.ToString()).ToList();
}

That's exactly the same scenario as trying to filter a set of TValue? to a set of TValue, even without the dictionary. It's just harder to see. TValue and TValue? code-blind us.

There are only two ways to do this. One is to copy each time, and the other is to keep two lists in synchronization.

Upvotes: 1

gnalck
gnalck

Reputation: 972

EDIT: If you want the latest source values, best way is to implement your own class that implements IReadOnlyDictionary. Initialize this with a private field of ReadOnlyDictionary<string, MyEnum?>. Each call will do the lookup, and if the key exists AND HasValue, return the value.

Note that this implementation depends on the reference to the original values being passed in as an IReadOnlyDictionary to avoid having to copy values over.

public class MyReadOnlyDictionary<TKey, TValue> : IReadOnlyDictionary<TKey, TValue> where TValue : struct
{
    // other methods to implement here...

    public MyReadOnlyDictionary(IReadOnlyDictionary<TKey, TValue?> kvps)
    {
        _kvps = kvps;
    }

    private IReadOnlyDictionary<TKey, TValue?> _kvps;

    new public TValue this[TKey key]
    {
        get
        {
            TValue? val = _kvps[key];
            if (val.HasValue)
                return val.Value;
            throw new KeyNotFoundException();
        }
    }
}

Upvotes: 1

Related Questions