Dice game rules implementation

My in-laws taught me a dice game a couple years ago, and we play every once in a while. A recent excellent answer from @radarbob inspired me to proceed with translating the rules of that dice game into code.

Here's what came out of it:

So it works perfectly, and somehow I'm just as unlucky with this virtual version as in the real-life one (had to click dozens of time to get a freakin' opening roll; by that time my mother-in-law already has thousands of points, every time). At least in this version I can inject some CrookedDie implementation if I want!

I'd like the CalculateRollScore method reviewed, to see what could be improved.

It turns out I thought the code was working as it should. So I wrote a couple unit tests and... well this is where not writing unit tests has bitten me.

Well I had to change the rules anyway, since the rule for 4x 1's was just my wife being mixed-up. My mother-in-law and I agree that 3x 1's is 1000, 4x 1's is 2000 and 5x 1's is 3000.

So before I show any code, I'll show this:

Now here's the working code (view original code here), with only minor changes that don't mootinize any already posted answer:

```
namespace DiceGame
{
public interface IRollScoreRules
{
int CalculateRollScore(IEnumerable> results);
}

public class GameRollScoreRules : IRollScoreRules
{
public virtual bool IsOpeningRoll(IEnumerable> results)
{
return CalculateRollScore(results) >= 500;
}

public virtual int CalculateRollScore(IEnumerable> results)
{
var score = 0;

// if less than 3 1's were rolled, each rolled 1 is 100pts:
score += results.GroupBy(e => e.Value)
.Where(g => g.Key == 1)
.Where(g => g.Count() g.Count() * 100);

// if less than 3 5's were rolled, each rolled 5 is 50pts:
score += results.GroupBy(e => e.Value)
.Where(g => g.

Well, I am not familiar with die, but the obvious improvement is to encapsulate those rules into separate entities. For example:

public interface IRollScoreRule
{
    int CalculateRollScore(IEnumerable> results);
}

public interface IRollScoreRules : IRollScoreRule
{
    bool IsOpeningRoll(IEnumerable> results);
    void AddRule(IRollScoreRule rule);
    void RemoveRule(IRollScoreRule rule);
}

public class ScoreFivesRule : IRollScoreRule
{
    int CalculateRollScore(IEnumerable> results)
    {
        // i think some of this logic can be extracted to base class
        // results.GroupBy(e => e.Value).Where(g => g.Key == 5) part for example
        // is common for most of the rules
        var score = results.GroupBy(e => e.Value)
                        .Where(g => g.Key == 5)
                        .Where(g => g.Count()  g.Count() * 50);
        return score;
    }
}

Then in your GameRollScoreRules.CalculateRollScore method you can iterate through collection of rules (using IEnumerable.Aggregate, for example) to get the total score, instead of having one huge method.

I also think you might want to tweak the IsOpeningRoll method. I mean, if I understand the use-case from your screenshots correctly, then with every dice roll, you want to know both - score value and if this value is higher then 500. Which means that using your GameRollScoreRules class as it is you would essentially calculate score twice (by calling CalculateRollScore and IsOpeningRoll methods). I think you sould either pass the resulting score to IsOpeningRoll method as a parameter, or remove this method all together and replace CalculateRollScore return type with some complex object (which will contain score value and bool flag) instead.

StackExchange Code Review Q#33363, answer score: 5