Best way to use a property to reference a Key-Value pair in a dictionary

Question

This is a fairly trivial matter, but I'm curious to hear people's opinions on it.

If I have a Dictionary which I'm access through properties, which of these formats would you prefer for the property?

/// <summary>
/// This class's FirstProperty property
/// </summary>
[DefaultValue("myValue")]
public string FirstProperty {
    get {
        return Dictionary["myKey"];
    }
    set {
        Dictionary["myKey"] = value;
    }

This is probably the typical way of doing it. It's fairly efficient, easy to understand, etc. The only disadvantage is with a longer or more complex key it would be possible to misspell it or change only one instance or something, leading me to this:

/// <summary>
/// This class's SecondProperty property
/// </summary>
[DefaultValue("myValue")]
private const string DICT_MYKEY = "myKey"
public string SecondProperty {
    get {
        return Dictionary[DICT_MYKEY];
    }
    set {
        Dictionary[DICT_MYKEY] = value;
    }

Which is marginally more complicated, but seems to offer additional safety, and is closer to what I would think of as the "Code Complete" solution. The downside is that when you also have a /// block and a [DefaultValue()] block above the property already, it starts getting a bit crowded up there.

So which do you like better, and why? Does anybody have any better ideas?

Answer 1

I like the second one purely because any avoidance of magic strings/numbers in code is a good thing. IMO if you need to reference a number or string literal in code more than once, it should be a constant. In most cases even if it's only used once it should be in a constant

Answer 2

I agree with @Glenn for a purely nit-picky point of view. The answer is whatever works for you. All this code takes place in 10 lines (if you include the omitted last curly brace). Nobody is going to get lost and the chance of mistyping is pretty slim (not impossible but very slim). On the other hand, if you used the key somewhere else, then DEFINATELY go with the constant.

Personally, I would go off on you about your curly brace style. :) Just kidding! It really is a matter of style.

Answer 3

This isn't answering your question, but I don't think "DefaultValue" means what you think it means. It doesn't set a default value for your property.

See MSDN and this question for more details.

Answer 4

A lot of people would probably argue that the second option is "correct", because any value used more than once should be refactored into a constant. I would most likely use the first option. You have already gotten close to the "Code Complete" solution by encapsulating the dictionary entry in a strong typed property. This reduces the chance of screwing up retrieving the wrong Dictionary entry in your implementation. There are only 2 places where you could mess up typing "myKey", in the getter and setter, and this would be very easy to spot.

The second option would just get too messy.

Answer 5

You could match the property names up to the keys and use reflection to get the name for the lookup.

public string FirstProperty {
get {
    return Dictionary[PropertyName()];
}
set {
    Dictionary[PropertyName()] = value;
}

private string PropertyName()
{
    return new StackFrame(1).GetMethod().Name.Substring(4);
}

This has the added benefit of making all your property implementation identical, so you could set them up in visual studio as code snippets if you want.

Answer 6

When you only use a magic string in one context, like you do, I think it's alright.
But if you ever need to use the key in another part of the class, go const.

Answer 7

@Joel you don't want to count on StackFrame. In-lining can ruin your day when you least expect it.

But to the question: Either way doesn't really matter a whole lot.

Answer 8

Why not use an enum?