Avoid using object to hold int, bool, double application setting values

Question

I have some code to handle integer, double, Boolean, and string settings. Right now, each setting variant extends an abstract class, where the setting value is stored as an object.

Is there any way to avoid this? What I'd like to do is have IntSetting objects return their Value as an integer, DoubleSetting objects return their Value as a double and so on and so forth.

Here is the code referenced above:

public abstract class ModelSetting
{
    public object Value { get; set; }
    public abstract string ToString();

    public ModelSetting(object value)
    {
        Value = value;
    }
}

public class IntSetting : ModelSetting
{
    public IntSetting(int value) 
        : base(value)
    {
    }

    public override string ToString()
    {
        return (bool)Value ? "1" : "0";
    }
}

public class DoubleSetting : ModelSetting
{
    public DoubleSetting(double value)
        : base(value)
    {
    }

    public override string ToString()
    {
        return ((double)Value).ToString("%g");
    }
}

Answer 1

As you say, returning an object isn't ideal. This is a relatively straightforward use case for generics:

public abstract class ModelSetting<T>
{
    public T Value { get; set; }
    public abstract string ToString();

    public ModelSetting(T value)
    {
        Value = value;
    }
}

Then implementing classes can use:

public class DoubleSetting : ModelSetting<double>
{
    //...The rest of the class
}

---

This:

public abstract string ToString();

Is bad. ToString is already defined on Object, so by declaring an abstract version here, you're not overriding, you're hiding. This is far less frequently done, and for good reason- it breaks our basic expectations of polymorphism. You should only do it with good reason, and you don't have one here. Just get rid of the ToString redeclaration, it gives you nothing.

---

You require a value in your constructor, but then let Value be set publicly. This doesn't really make much sense- somebody could just pass default(T) to the constructor, making the constructor meaningless. You're giving two different ways to set the value, for no particular reason. It's not a big deal, but is needlessly confusing.

Consider whether you actually need to be able to change Value for an existing ModelSetting instance. If so, remove the constructor. If not (and this is probably better!), make the property private set. This makes the value immutable, which makes it much easier to reason about the class. For example, if I hold an instance, I know I can pass that instance to whoever I want without having to worry about them changing the value without me expecting it.

---

return (bool)Value ? "1" : "0";

Shouldn't this be the BoolSetting version, rather than the IntSetting one? At least you can take comfort that this bug is a demonstration of one of the advantages of the strong-typedness that you get from the generic approach!