Chaining statistics in a dice queue

Question

I've previously came up with the question "Rolling dice in a method chain" and was interested if this could be improved. It could, so now I have this class:

public class DiceQueue : IEnumerable<int>
{
    public int Sides { get; }
    internal Random Rnd = new Random();
    public DiceQueue(int sides) { Sides = sides; }

    public int Roll() => Rnd.Next(Sides) + 1;
    public IEnumerator<int> GetEnumerator() { while (true) { yield return Roll(); } }
    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

Very simple: just create a queue by calling the constructor with a number of sides and then it can be used as an infinite enumerator. But generally, you just want to take an X amount of values from the queue at once, or even just roll a single die. But this class isn't that interesting, as I want to do statistics with random rolls!
So, hence this class:

public class Statistics : IEnumerable<Point<long>>
{
    public string Name { get; private set; }
    public long Sum { get; private set; } = 0;
    public long Total { get; private set; } = 0;
    public long Min { get; private set; } = long.MaxValue;
    public long Max { get; private set; } = long.MinValue;
    public double Average => 1.0 * Sum / Total;
    public double TotalOf(int value) => Totals.ContainsKey(value) ? 100.0 * Totals[value] / Total : 0.0;
    protected Dictionary<long, long> Totals { get; set; } = new Dictionary<long, long>();
    public event EventHandler<StatisticsEventArgs> Added;
    protected virtual void OnAdded(StatisticsEventArgs e) { Added?.Invoke(this, e); }
    public Statistics() : this("No name") { }
    public Statistics(string name) { Name = name; }
    public Statistics Add(long value)
    {
        if (!Totals.ContainsKey(value)) { Totals.Add(value, 0); }
        Totals[value] += 1;
        Total++;
        Sum += value;
        if (Max < value) { Max = value; }
        if (value < Min) { Min = value; }
        OnAdded(new StatisticsEventArgs(this, value));
        return this;
    }
    public IEnumerable<string> Report()
    {
        yield return $"Report for: {Name}";
        yield return $"The total sum is {Sum} for {Total} values for an average value of {Average:0.00}.";
        yield return $"The values are all between {Min} and {Max}.";
        yield return "List of values";
        foreach (var point in this) { yield return $"* Value {point.X} occurred {point.Y} times: {100.0 * point.Y / Total:00.0}%."; }
        yield return new string('-', 40);
    }
    public override string ToString() => string.Join(Environment.NewLine, Report());
    public IEnumerator<Point<long>> GetEnumerator()
    {
        foreach (var total in Totals.OrderBy(pair => pair.Key)) { yield return new Point<long> { X = total.Key, Y = total.Value }; }
    }
    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

It has a name and you just add values to it and it will keep various statistics about it. The Report() method will generate a textual report of the summary but it's mainly used for the ToString() method. Interesting way to make multiline reports, btw.
It also has an event which gets triggered whenever a new value is added, allowing the system to display a running total.
I have to show a few more types, though. First:

public class Point<T>
{
    public T X { get; internal set; }
    public T Y { get; internal set; }
}

Not really rocket science. And while C# does have a Point datatype somewhere, I wanted one that's more generic...

public class StatisticsEventArgs : EventArgs
{
    public Statistics Stats { get; }
    public long LastValue { get; }
    public StatisticsEventArgs(Statistics stats, long lastValue)
    {
        Stats = stats;
        LastValue = lastValue;
    }
}

Yeah, well... The event requires an argument. This one should be it. Trying to follow the standard for events here.
But I want to use the statistics in a chainable way and I want to do statistics on rolls of multiple dice, so I also have these extension methods:

public static class Toolkit
{
    public static IEnumerable<T> Do<T>(this IEnumerable<T> data, Action<T> action)
    {
        foreach (var item in data)
        {
            action(item);
            yield return item;
        }
    }
    public static IEnumerable<List<T>> TakeGroup<T>(this IEnumerable<T> data, int count)
    {
        while (true) { yield return data.Take(count).ToList(); }
    }
}

The Do() method just does some action with a value from an enumerator before passing the value on to the next method. Maybe not elegant, but I have no other options here, as far as I know.
The TakeGroup() method will grab an X amount of values and return them as a list, and keeps doing this until infinity. Too bad that this might cause trouble if the base enumeration is limited so I need to work out a solution for that. A Partitioner, perhaps? Well, still have to work that out but this works for my current purpose...
Then something to test the whole thing.

public static class StatisticsTest
{
    public static void Execute(int count)
    {
        Console.WriteLine($"We've rolled for a total of {D6X3RollQueue.Take(count).Count()} times.");
        Console.WriteLine(D6Stat);
        Console.WriteLine(D6X3MinStat);
        Console.WriteLine(D6X3MaxStat);
        Console.WriteLine(D6X3MinMaxStat);
        Console.WriteLine(D6X3SumStat);
    }
    private static readonly DiceQueue D6 = new DiceQueue(6);
    private static readonly Statistics D6Stat = new Statistics("Rolling d6");
    private static readonly Statistics D6X3MinStat = new Statistics("Minimum of 3D6 (1 to 6)");
    private static readonly Statistics D6X3MaxStat = new Statistics("Maximum of 3D6 (1 to 6)");
    private static readonly Statistics D6X3MinMaxStat = new Statistics("Sum(Min+Max) of 3D6 (2 to 12)");
    private static readonly Statistics D6X3SumStat = new Statistics("Sum of 3D6 (3 to 18)");
    private static readonly IEnumerable<int> D6RollQueue = D6.Do(d => D6Stat.Add(d));
    private static readonly IEnumerable<int> D6X3RollQueue = D6RollQueue.TakeGroup(3)
                                                                        .Do(d => D6X3MinStat.Add(d.Min()))
                                                                        .Do(d => D6X3MaxStat.Add(d.Max()))
                                                                        .Do(d => D6X3MinMaxStat.Add(d.Min() + d.Max()))
                                                                        .Select(d => d.Sum())
                                                                        .Do(d => D6X3SumStat.Add(d));

}

This shows why I want a dice queue. The queue allows me to collect various statistics for all the rolls made while I can roll as many times as I like. By declaring a queue like D6RollQueue I can take a single roll by using D6RollQueue.First() or an X amount of rolls by using `D6RollQueue.Take(X)' and then do something with the result, while the queue keeps track of all statistics. So when I create some dice game, I can just roll as often as I like while having various statistics about the rolls.
And the TakeGroup() method will allow me to maintain statistics about groups of rolls like rolling three dice and keep statistics about the minimum and maximum values.

So, go ahead and shoot at it. Can it be improved?
(It's not that important that the Random generator might be predictable.)

Answer 1

• If you need to get a value of a Dictionary<TKey, TValue> you shouldn't use ContainsKey() together with the Item property getter but TryGetValue(), because by using ContainsKey() in combination with the Item getter you are doing the check if the key exists twice.
  From the refernce source

  ContainsKey(TKey)

  public bool ContainsKey(TKey key) {
      return FindEntry(key) >= 0;
  }
  

  this[TKey]

  public TValue this[TKey key] {
      get {
          int i = FindEntry(key);
          if (i >= 0) return entries[i].value;
          ThrowHelper.ThrowKeyNotFoundException();
          return default(TValue);
      }
      set {
          Insert(key, value, false);
      }
  }
  

  TryGetValue(TKey, out TValue)

  public bool TryGetValue(TKey key, out TValue value) {
      int i = FindEntry(key);
      if (i >= 0) {
          value = entries[i].value;
          return true;
      }
      value = default(TValue);
      return false;
  }
  

  Each of these public methods is calling the private FindEntry(TKey). As you see a combination of ContainsKey() together with the Item getter is doing the call twice.

  private int FindEntry(TKey key) {
      if( key == null) {
          ThrowHelper.ThrowArgumentNullException(ExceptionArgument.key);
      }
  
      if (buckets != null) {
          int hashCode = comparer.GetHashCode(key) & 0x7FFFFFFF;
          for (int i = buckets[hashCode % buckets.Length]; i >= 0; i = entries[i].next) {
              if (entries[i].hashCode == hashCode && comparer.Equals(entries[i].key, key)) return i;
          }
      }
      return -1;
  }
  

  So this

  if (!Totals.ContainsKey(value)) { Totals.Add(value, 0); }
  Totals[value] += 1;  
  

  should be written like this

  Totals.TryGetValue(value, out long current);
  Totals[value] = current + 1;
  

  But just looking at the above code, it just seems strange to have ContainsKey(value). It would be better to rename value to something else.

• I don't like such oneliners shown in the code. IMO it is harder to read the code and grasp at first glance what it is about.

• The extension methods could use some improvement as well. You should add proper argument validation into the methods. Nobody wants to get a NullReferenceException out of a public method.

• The Name property doesn't need a setter because you are assigning a value only in the constructor.

Answer 2

• The number of times a given value has been added to a statistics batch shouldn't be a long, it should be an int (maybe unsigned). (I was mistaken about what long was).
• That would seem to break your Point implementation, but you probably shouldn't be using that in the first place. The items in question aren't "points"; they don't spatially relate to each other. Just use tuples.
• The DiceQueue class yields ints, so it would be ideal if Statistics also handled ints. Can you make Statistics generic across numerical types?
• I suspect that you can shorten/simplify a lot of this using Linq, maybe to the point where it no longer needs to be wrapped up in classes the way you have it. But I don't know what that would look like exactly.

It's hard to talk about the ideal way to do what you're trying to do without knowing what you're working toward. How will these tools be used?