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

A simple testable standalone DLL for die rolling

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

Problem

I've decided to challenge myself by writing a test standalone DLL with the objective of making all of its publicly-exposed objects able to be as intuitive and as simple-to-use as possible.

I've decided to try and create some objects that might be used in a game, like a deck of cards and a die.

Here's my Die for example:

public class Die
{
    private readonly Random _random;
    private readonly int _numberOfSides;

    public Die(int numberOfSides)
    {
        if(numberOfSides < 2)
            throw new ArgumentException(string.Format(GameComponentsResources.InvalidNumberOfSides, numberOfSides));

        _numberOfSides = numberOfSides;
        _random = new Random();
    }

    public int Roll()
    {
        return _random.Next(1, _numberOfSides);
    }
}


So obviously the random part of this is untestable unless I write a non-deterministic test, which I don't want to do.

If this was standard enterprise code I was writing, I'd mock the Random and inject it. However, let's say someone shelled out some cash for my GameComponents library. Do I think they would consider it a good product if they have to instantiate and pass in a random just to get a die for their game? Nah. I'd rather keep it concise and intuitive, like this: var die = new Die(6); or at the very most var die = dieCollection.GetDie(6);

So standard constructor injection is out of the picture for now. Property injection is better, but I still don't want to start depending on a container and writing a bootstrapper for something that is basically encapsulating a random.

I was thinking about getting hold of a seed value and passing it around (but without forcing consumers to pass in a seed value, unless they really wanted to), so I can get some deterministic values for my test, but the only way I can think to do this while maintaining good encapsulation is making a method like this:

```
protected virtual long GetSeed() //could be a property too
{
return Environment.TickCount;

Solution

Add one more constructor that takes an extra Random param:

public Die(int numberOfSides, Random random)
{
    if(numberOfSides < 2)
        throw new ArgumentException(string.Format(GameComponentsResources.InvalidNumberOfSides, numberOfSides));

    _numberOfSides = numberOfSides;
    _random = random;
}

public Die(int numberOfSides)
{
    this(numberOfSides, new Random());
}


Most users will use the single-param version.
Some users, who want more control, for example in tests, can use the two-param version.
You can write your tests in a straightforward way using the two-param version.

If you really don't want to expose the two-param constructor in the public API,
then make it protected.
But I think it's quite fine to include in the public API,
it doesn't make the library confusing, and I don't see any harm.

If, for some reason you are against passing a Random instance (like @JasonLind, see his comment),
then you can make the constructor taking a Random above private,
and add another constructor that takes a seed parameter instead:

public Die(int numberOfSides, int seed)
{
    this(numberOfSides, new Random(seed));
}

Code Snippets

public Die(int numberOfSides, Random random)
{
    if(numberOfSides < 2)
        throw new ArgumentException(string.Format(GameComponentsResources.InvalidNumberOfSides, numberOfSides));

    _numberOfSides = numberOfSides;
    _random = random;
}

public Die(int numberOfSides)
{
    this(numberOfSides, new Random());
}
public Die(int numberOfSides, int seed)
{
    this(numberOfSides, new Random(seed));
}

Context

StackExchange Code Review Q#82862, answer score: 5

Revisions (0)

No revisions yet.