I've built the groundwork for an attributes (as in Strength, Intelligence, not as in DebuggerHidden, TestMethod) framework for my game. Each attribute can be upgraded and downgraded individually as well as part of a bulk change (such as for upgrades that have side effects, or leveling up).I'm looking to get a thorough critique of the groundwork before I go expanding on shaky foundations.UpgradeableStat and UpgradeableStat<T>These are the absolute bases of the attributes system. UpgradeableStat only really exists so as to allow me to hold a collection of UpgradeableStat instances with different T types, all attributes will really extend from UpgradeableStat<T>public abstract class UpgradeableStat{    public abstract void Upgrade();    public abstract void Downgrade();}public abstract class UpgradeableStat<T> : UpgradeableStat{    public abstract T Value    {        get;    }}OneShotStatThese attributes will be applied once, until it's been removed (whereby it's applied again). Think of it as a door, you can't open it again unless it's been closed./// <summary>/// A statistic that is upgraded only once until it is next removed./// </summary>public class OneShotStat<T> : UpgradeableStat<T>{    /// <summary>    /// Gets the value.    /// </summary>    /// <value>    /// The value.    /// </value>    public override T Value    {        get        {            return Upgraded ? UpgradedValue : DowngradedValue;        }    }    /// <summary>    /// Gets or sets the upgraded value.    /// </summary>    /// <value>    /// The upgraded value.    /// </value>    public T UpgradedValue    {        get;        set;    }    /// <summary>    /// Gets or sets the downgraded value.    /// </summary>    /// <value>    /// The downgraded value.    /// </value>    public T DowngradedValue    {        get;        set;    }    /// <summary>    /// Gets a value indicating whether this <see cref=OneShotStat/> is upgraded.    /// </summary>    /// <value>    ///   <c>true</c> if upgraded; otherwise, <c>false</c>.    /// </value>    public bool Upgraded    {        get;        private set;    }    /// <summary>    /// Upgrades this instance if it is not already upgraded.    /// </summary>    public override void Upgrade()    {        if (!Upgraded)        {            Upgraded = true;        }    }    /// <summary>    /// Downgrades this instance if it is not already downgraded.    /// </summary>    public override void Downgrade()    {        if (Upgraded)        {            Upgraded = false;        }    }}LevelThresholdOneShotStatThis is a fairly common attribute type. The attribute is set to the level up value only when the attribute's level is raised above a certain threshold. This is good for attributes that have a super form, for example./// <summary>/// A OneShotStat that is applied when the stat breaches a level threshold and is removed when it sinks below it./// </summary>public class LevelThresholdOneShotStat<T> : OneShotStat<T>{    /// <summary>    /// The level    /// </summary>    private int level;    /// <summary>    /// Gets the level.    /// </summary>    /// <value>    /// The level.    /// </value>    public int Level    {        get        {            return level;        }        private set        {            level = Math.Max(MinLevel, Math.Min(level, MaxLevel));            if (Level >= Threshold)            {                base.Upgrade();            }            else            {                base.Downgrade();            }        }    }    /// <summary>    /// The threshold    /// </summary>    private int threshold;    /// <summary>    /// Gets or sets the threshold.    /// </summary>    /// <value>    /// The threshold.    /// </value>    /// <exception cref=System.ArgumentOutOfRangeException>    /// Threshold;Argument must not be greater than MaxLevel    /// or    /// Threshold;Argument must not be less than MinLevel    /// </exception>    public int Threshold    {        get        {            return threshold;        }        set        {            if (value > MaxLevel)            {                throw new ArgumentOutOfRangeException(Threshold, Argument must not be greater than MaxLevel);            }            if (value < MinLevel)            {                throw new ArgumentOutOfRangeException(Threshold, Argument must not be less than MinLevel);            }            threshold = value;            Level = Level;        }    }    /// <summary>    /// The minimum level    /// </summary>    private int minLevel;    /// <summary>    /// Gets or sets the minimum level.    /// </summary>    /// <value>    /// The minimum level.    /// </value>    /// <exception cref=System.ArgumentOutOfRangeException>MinLevel;Argument must not be greater than MaxLevel</exception>    public int MinLevel    {        get        {            return minLevel;        }        set        {            if (value > MaxLevel)            {                throw new ArgumentOutOfRangeException(MinLevel, Argument must not be greater than MaxLevel);            }            minLevel = value;            if (Level < MinLevel)            {                Level = MinLevel;            }        }    }    /// <summary>    /// The maximum level    /// </summary>    private int maxLevel;    /// <summary>    /// Gets or sets the maximum level.    /// </summary>    /// <value>    /// The maximum level.    /// </value>    /// <exception cref=System.ArgumentOutOfRangeException>MaxLevel;Argument must not be less than MinLevel</exception>    public int MaxLevel    {        get        {            return maxLevel;        }        set        {            if (value < MinLevel)            {                throw new ArgumentOutOfRangeException(MaxLevel, Argument must not be less than MinLevel);            }            maxLevel = value;            if (Level > MaxLevel)            {                Level = MaxLevel;            }        }    }    /// <summary>    /// Upgrades this instance.    /// </summary>    public override void Upgrade()    {        Level++;    }    /// <summary>    /// Downgrades this instance.    /// </summary>    public override void Downgrade()    {        Level--;    }}InterpolatedUpgradeableStatThis is probably the most common type of attribute. It starts at one value and increases with each upgrade up to a maximum./// <summary>/// Upgradeable stat whose value is set by interpolating between a minimum and maximum value based on the stat's level./// </summary>/// <typeparam name=T>Type of internal value of the statistic</typeparam>public class InterpolatedUpgradeableStat<T> : UpgradeableStat<T>{    /// <summary>    /// Gets the value.    /// </summary>    /// <value>    /// The value.    /// </value>    public override T Value    {        get        {            return interpolator.Interpolate(Min, Max, (float)Level / (float)upgradeSteps);        }    }    /// <summary>    /// The interpolator    /// </summary>    private IInterpolator<T> interpolator;    /// <summary>    /// Gets or sets the interpolator.    /// </summary>    /// <value>    /// The interpolator.    /// </value>    /// <exception cref=System.ArgumentNullException>Interpolator</exception>    public IInterpolator<T> Interpolator    {        get        {            return interpolator;        }        set        {            if (value == null)            {                throw new ArgumentNullException(Interpolator);            }            interpolator = value;        }    }    /// <summary>    /// The upgrade steps    /// </summary>    private int upgradeSteps;    /// <summary>    /// Gets or sets the upgrade steps.    /// </summary>    /// <value>    /// The upgrade steps.    /// </value>    /// <exception cref=System.ArgumentOutOfRangeException>UpgradeSteps;Argument must be not be less than 0.</exception>    public int UpgradeSteps    {        get        {            return upgradeSteps;        }        set        {            if (value < 0)            {                throw new ArgumentOutOfRangeException(UpgradeSteps, value, Argument must be not be less than 0.);            }            upgradeSteps = value;        }    }    /// <summary>    /// The maximum    /// </summary>    private T max;    /// <summary>    /// Gets or sets the maximum.    /// </summary>    /// <value>    /// The maximum.    /// </value>    /// <exception cref=System.ArgumentOutOfRangeException>UpgradeSteps;Argument must be not be less than Min.</exception>    public T Max    {        get        {            return max;        }        set        {            if (value is IComparable && ((IComparable)value).CompareTo(Min) < 0)            {                throw new ArgumentOutOfRangeException(UpgradeSteps, value, Argument must be not be less than Min.);            }            max = value;        }    }    /// <summary>    /// The minimum    /// </summary>    private T min;    /// <summary>    /// Gets or sets the minimum.    /// </summary>    /// <value>    /// The minimum.    /// </value>    /// <exception cref=System.ArgumentOutOfRangeException>UpgradeSteps;Argument must be not be greater than Max.</exception>    public T Min    {        get        {            return min;        }        set        {            if (value is IComparable && ((IComparable)value).CompareTo(Max) > 0)            {                throw new ArgumentOutOfRangeException(UpgradeSteps, value, Argument must be not be greater than Max.);            }            min = value;        }    }    /// <summary>    /// Initializes a new instance of the <see cref=InterpolatedUpgradeableStat{T}/> class.    /// </summary>    /// <param name=min>The minimum.</param>    /// <param name=max>The maximum.</param>    /// <param name=upgradeSteps>The upgrade steps.</param>    /// <param name=interpolator>The interpolator.</param>    public InterpolatedUpgradeableStat(T min, T max, int upgradeSteps, IInterpolator<T> interpolator)    {        this.Min = min;        this.Max = max;        this.UpgradeSteps = upgradeSteps;        this.Interpolator = interpolator;    }    /// <summary>    /// Gets the level.    /// </summary>    /// <value>    /// The level.    /// </value>    public int Level    {        get;        private set;    }    /// <summary>    /// Upgrades this instance.    /// </summary>    public override void Upgrade()    {        //Level range is 0..upgradeSteps-1        Level = Math.Min(upgradeSteps - 1, Level + 1);    }    /// <summary>    /// Downgrades this instance.    /// </summary>    public override void Downgrade()    {        Level = Math.Max(0, Level - 1);    }}
Lvl 1 upgradeable attributes
c#;game
You know, code wise there is not really much to do. The other comments covered the few ways to improve things. I would though suggest a look at your goals architecturally.Aside from the semantically different meaning of an upgradable vs downgradable attribute both being under the same umbrella being a problem, in theory  I don't see why an upgradable attribute is a subset of attribute. In theory any location you want to use an attribute it should also be swappable by an modified attribute right?Also Upgrading and Downgrading are two specific use cases.at some stage you may want to freeze an attributes values, or make a combined attribute or have a modifier only available if a condition is met. All of that would require some level of a redesign. If you instead looked at an attribute as a base component:IAttribute<T>{  T Value{ get; }}and a modification as a completely different thing:IModifier<T>{  T Modify(T item)  bool IsActive {get;set;}}You can now create an upgradable attribute but have your consuming code agnostic of its base type. Then, you have scope for some other changes:ModifiedAttribute<T> : IAttribute<T>{    IList<IModifier<T>> Modifiers{ get; }    bool UseModifiers{ get; }    T Value     {   // here you might return a base value, but if use modifiers is selected and there are any, loop through and apply them.     }}This same separation of concerns can now be applied directly to the modifiers. Picture adding a conditional modifier, that only applies if the player has certain buffs or attributes, such asreturn (Player.Buffs.Contains(Poison Resistant))or a weighted Modifier, which is just a modifier with an Int value, now you can sort based on that value and apply the modifiers in order based on their weight value. still agnostic from the Attribute. or whatever.... and conditionally apply these modifiers.The possibilities are endless, In short, when architecting things, try to distill to the lowest base type and establish what consuming code really needs to know, absolute minimum, to avail of it's functionality. Any additional functionality can always be passed in through constructor arguments or public properties on an implementation. 