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

Automatic computation of mean, standard-deviation, sum, min, max in property

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

Problem

Working on a game I need to record events compute some values about it (the more the better). The twist is, I need to store as little data as possible. Constant (as in O(1)) weight is required. The following code is not acceptable :

public class Player
{
    private List previousGamesDuration;//Too heavy
    float GetAvgGameTime ();
    float GetStdDevGameTime ();
    float GetMaxGameTime ();
    ...
}


So I ended up having the following :

public class Player
{
    public MagicFloat GameDuration;
}


With the MagicFloat being a subclass of MagicNumber as following.

```
namespace network
{
[Serializable]
public abstract class MagicProperty where T : IComparable
{
protected T _last;

public T Last
{
get { return _last; }
set
{
if (Count == 0)
{
Min = value;
Max = value;
}
Min = value.CompareTo (Min) 0 ? value : Max;
Count++;
UpdateSums (value);
UpdateSigma ();
UpdateAverage ();
_last = value;
}
}

public override string ToString ()
{
return string.Format ("Last: {0}, Max: {1}, Min: {2}, Avg: {3}, Count: {4}, Sum: {5}, Sum2: {6}, Sigma: {7}", Last,
Max, Min, Avg, Count, Sum, Sum2, Sigma);
}

protected abstract void UpdateSigma ();

protected abstract void UpdateSums (T value);

protected abstract void UpdateAverage ();

public T Max { get; private set; }

public T Min { get; private set; }

public float Avg { get; protected set; }

public int Count { get; private set; }

public T Sum { get; protected set; }

public T Sum2 { get; protected set; }

public float Sigma { get; protected set; }

// ReSharper disable once MemberCanBePrivate.Global

Solution

You can add additional constraint to your generic type :

public abstract class MagicProperty 
        where T : struct, IComparable


It doesn't makes sense to have a public constructor in an abstract class, since abstract classes are instantiated through they're derived types, more appropriate access modifier would be protected.

public MagicProperty(T initialValue)


There is no need of 2 constructors either as you can use optional parameters and have just a single constructor :

protected MagicProperty(T initialValue = default(T))
{
    if (!EqualityComparer.Default.Equals(initialValue, default(T)))
    {
        Last = initialValue;
    }
}


But repeating the absolute same code in all of your derived types constructors is not a nice thing. You should instead inherit the base constructor like this :

protected MagicInt(int initialValue = default(int)) : base(initialValue)
{
}

protected MagicFloat(float initialValue = default(float)) : base(initialValue)
{
}


A lot of your code has repetitive implementation in the derived class and this is due to the generic type parameter since it can't be restricted to numbers only thus apply arithmetic operations to them.

What you can do in this case is use dynamic instead. I would keep the type argument just so you can be sure that your class always operates on the exact same type.

BUT in order for this to work you will need to explicitly specify the type of the value you are assigning to Last :

MagicProperty GameDuration = new MagicProperty();
GameDuration.Last = 123;


You can't do that since the type of 123 right now looks like an Int32, instead of float the proper way of doing this is :

MagicProperty GameDuration = new MagicProperty();
GameDuration.Last = 123f; // <---- f


And also assign 0 to Sum, Sum2, Min, Max in your ctor.

Let's start by changing all of your properties and fields types :

protected dynamic _last;

public dynamic Last
{
    get { return _last; }
    set
    {
        //..
    }
}

public dynamic Max { get; private set; }

public dynamic Min { get; private set; }

public float Avg { get; protected set; }

public int Count { get; private set; }

public dynamic Sum { get; protected set; }

public dynamic Sum2 { get; protected set; }

public float Sigma { get; protected set; }


Next we need to have a way of verifying if the dynamic type matches the original type argument T, let's create a short function to do that for us :

private bool IsSameTypeAsT(Type type)
{
    return type == typeof(T);
}


Since Last is publicly settable we need to have additional check there for the type :

if (!IsSameTypeAsT(value.GetType()))
{
    throw new InvalidCastException("Types don't match");
}


Next let's replace those lines :

Min = value.CompareTo (Min)  0 ? value : Max;


With the predefined methods in the Math class :

Min = Math.Min(value, Min);
Max = Math.Max(value, Max);


This check here :

if (Count == 0)
{
    Min = value;
    Max = value;
}


Is redundant as you're overriding that value on the next line.

Now your property setter looks like this :

set
{
    if (!IsSameTypeAsT(value.GetType()))
    {
        throw new InvalidCastException("Types don't match");
    }
    Min = Math.Min(value, Min);
    Max = Math.Max(value, Max);
    Count++;
    UpdateSums(value);
    UpdateSigma();
    UpdateAverage();
    _last = value;
}


Next we have the abstract UpdateSums(...) method which is implemented the same way in both of your derived types. Let's implement it just once in the base class like this :

protected void UpdateSums(dynamic value)
{
    if (!IsSameTypeAsT(value.GetType()))
    {
        throw new InvalidCastException("Types don't match");
    }
    Sum += value;
    Sum2 += value * value;
}


We also have abstract UpdateAverage() which is implemented again the same way in both derived types :

protected void UpdateAverage()
{
    Avg = (float) decimal.Divide((decimal) Sum, Count);
}


Or you can simply do :

protected void UpdateAverage()
{
    Avg = (float) Sum / Count;
}


Lastly we have abstract UpdateSigma() :

protected void UpdateSigma()
{
    float meanSquared = Avg * Avg;
    Sigma = (float)Math.Sqrt((float)decimal.Divide((decimal)Sum2, Count) - meanSquared);
}


Having all of those methods implemented in your base class, makes all of your derived types useless. Which means you can now have a single non-abstract class which does the job for you, with some final property access modifiers changes your class can look like this :

```
[Serializable]
public class MagicProperty
where T : struct, IComparable
{
private dynamic _last;
public dynamic Last
{
get { return _last; }
set
{
if (!IsSameTypeAsT(value.GetType()))
{
throw new InvalidCastException("Types don't match");
}
Min = Math.

Code Snippets

public abstract class MagicProperty<T> 
        where T : struct, IComparable
public MagicProperty(T initialValue)
protected MagicProperty(T initialValue = default(T))
{
    if (!EqualityComparer<T>.Default.Equals(initialValue, default(T)))
    {
        Last = initialValue;
    }
}
protected MagicInt(int initialValue = default(int)) : base(initialValue)
{
}

protected MagicFloat(float initialValue = default(float)) : base(initialValue)
{
}
MagicProperty<float> GameDuration = new MagicProperty<float>();
GameDuration.Last = 123;

Context

StackExchange Code Review Q#152311, answer score: 8

Revisions (0)

No revisions yet.