0

This is my first question on StackOverflow. I have a work-related problem which I have rewritten to be about Mario. Me and my colleagues cannot come up with an elegant solution and I am wondering if you have any ideas to help us out.

The problem:
The switch case below is one of many switch cases in our code. The one below is one of the more simple cases we use. In this case, there are 3 levels * 5 enemies = 15 combinations. All combinations have a specific expectedDifficulty.

We prefer to test all possibilities, as this is the safest approach for reliability. We think this is necessary to prevent software developers to suddenly say "I'm gonna introduce a new difficulty, namely Medium, for Enemy C in DesertLand."

There are cases in our code where we might have over 100 possibilities. At some point this is gonna get very uncomfortable (even with 15 possibilities I'm feeling uncomfortable).

Possible solutions:
#1 one method per level. Advantage: 5 TestCases per test. Disadvantage: duplicate code.
#2 foreach methods. Advantage: more compact code. Disadvantage: no overview in the combinations and the expected result.

My question:
Does anybody have a good idea to elegantly solve this? Or should we not want to test all possibilities? If so? Could you explain why and why you don't do this at your work?

I am looking forward to reading suggestions.

The main code:
The following method sets the difficulty based on the level and the inputModel:

[assembly: InternalsVisibleTo("MarioTestBase")]
internal class Mario 
{
    internal void SetDifficulty(InputModel inputModel)
    {
        switch (this.Level)
        {
            default:
                throw new ArgumentException("This level does not exist.");
            case Level.GrassLand:
                {
                    this.Difficulty = Difficulty.Basic;
                    break;
                }
            case Level.DesertLand:
                {
                    switch (inputModel.Enemy)
                    {
                        default:
                            throw new ArgumentException("Enemy does not exist.");
                        case Enemy.A:
                        case Enemy.B:
                        case Enemy.C:
                        case Enemy.D:
                        case Enemy.E:
                            {
                                this.Difficulty = Difficulty.Basic;
                                break;
                            }
                    }

                    break;
                }
            case Level.WaterLand:
                {
                    switch (inputModel.Enemy)
                    {
                        default:
                            throw new ArgumentException("Enemy does not exist.");
                        case Enemy.A:
                        case Enemy.B:
                        case Enemy.C:
                            {
                                this.Difficulty = Difficulty.Advanced;
                                break;
                            }
                        case Enemy.D:
                        case Enemy.E:
                            {
                                this.Difficulty = Difficulty.Basic;
                                break;
                            }
                    }

                    break;
                }
        }
    }
}

The test:
I have written the following test. The TestCases are based on two Enums (Level and Enemy) and have an expected Enum, namely Difficulty.

[TestCase(Level.GrassLand, Enemy.A, Difficulty.Basic)]
[TestCase(Level.GrassLand, Enemy.B, Difficulty.Basic)]
[TestCase(Level.GrassLand, Enemy.C, Difficulty.Basic)]
[TestCase(Level.GrassLand, Enemy.D, Difficulty.Basic)]
[TestCase(Level.GrassLand, Enemy.E, Difficulty.Basic)]
[TestCase(Level.DesertLand, Enemy.A, Difficulty.Basic)]
[TestCase(Level.DesertLand, Enemy.B, Difficulty.Basic)]
[TestCase(Level.DesertLand, Enemy.C, Difficulty.Basic)]
[TestCase(Level.DesertLand, Enemy.D, Difficulty.Basic)]
[TestCase(Level.DesertLand, Enemy.E, Difficulty.Basic)]
[TestCase(Level.WaterLand, Enemy.A, Difficulty.Advanced)]
[TestCase(Level.WaterLand, Enemy.B, Difficulty.Advanced)]
[TestCase(Level.WaterLand, Enemy.C, Difficulty.Advanced)]
[TestCase(Level.WaterLand, Enemy.D, Difficulty.Basic)]
[TestCase(Level.WaterLand, Enemy.E, Difficulty.Basic)]
public class SetDifficulty_Tests : MarioTestBase
{
    public void SetDifficulty_ShouldSelectCorrectDifficulty(Level level, Enemy enemy, Difficulty expectedDifficulty)
    {
        // Arrange
        Mock<Mario> _mock = new Mock<Mario>();
        _mock.Setup(x => x.Level).Returns(level);
        testModel.Enemy = enemy;

        // Act
        _mock.Object.SetDifficulty(testModel);
        Difficulty actualDifficulty = _mock.Object.Difficulty;

        // Assert
        Assert.AreEqual(expectedDifficulty, actualDifficulty);
    }
}

The following SetUp is used in a centralized class:

public class MarioTestBase
{
    protected IMario Mario;

    public InputModel testModel;

    [SetUp]
    public void Setup()
    {
        Mario = new Mario();
        testModel = new InputModel();
    }
}

An example implementation of the foreach solution, which I don't like:

[TestCase(Difficulty.Basic)]
[TestCase(Difficulty.Basic)]
[TestCase(Difficulty.Basic)]
[TestCase(Difficulty.Basic)]
[TestCase(Difficulty.Basic)]
[TestCase(Difficulty.Basic)]
[TestCase(Difficulty.Basic)]
[TestCase(Difficulty.Basic)]
[TestCase(Difficulty.Basic)]
[TestCase(Difficulty.Basic)]
[TestCase(Difficulty.Advanced)]
[TestCase(Difficulty.Advanced)]
[TestCase(Difficulty.Advanced)]
[TestCase(Difficulty.Basic)]
[TestCase(Difficulty.Basic)]
public class SetDifficulty_Tests : MarioTestBase
{
    public void SetDifficulty_ShouldSelectCorrectDifficulty(Difficulty expectedDifficulty)
    {
        // Arrange
        Mock<Mario> _mock = new Mock<Mario>();
        _mock.Setup(x => x.Level).Returns(level);
        testModel.Enemy = enemy;
        
        foreach (Level level in Enum.GetValues(typeof(Level)))
        {
            _mock.Setup(x => x.Level).Returns(level);
            
            foreach (Enemy enemy in Enum.GetValues(typeof(Enemy)))
            {
                testModel.Enemy = enemy;
                _mock.Object.SetDifficulty(inputModel);
                Difficulty actualDifficulty = _mock.Object.Difficulty;
                Assert.AreEqual(expectedDifficulty, actualDifficulty);
            }
        }       
    }
}
Woudjee
  • 1
  • 1

1 Answers1

0

I think your question is too generic. As for the elegance of the solution, I'd prefer to use a 3D matrix of Level. Enemy, Difficulty (which could be implemented by a Dictionary<Level,Dictionary<Enemy,Difficulty>>) instead all of those switches. Here it is an example:

 [assembly: InternalsVisibleTo("MarioTestBase")]
 internal class Mario 
 {

      private final IDictionary<Level,IDictionary<Enemy,Difficulty>> _difficultyMatrix;
 
      internal Mario(IDictionary<Level,IDictionary<Enemy,Difficulty>> difficultyMatrix) {
           _difficultyMatrix = difficultyMatrix;
      }

      internal void SetDifficulty(InputModel inputModel)
      {
           if (!_difficultyMatrix.ContainsKey(this.Level))
           {
               throw new ArgumentException("Level doesn't exist");
           }
           if (!_difficultyMatrix[this.Level].ContainsKey(inputModel.Enemy))
           {
               throw new ArgumentException("Enemy doesn't exist");
           }
           this.Difficulty = _difficultyMatrix[this.Level][inputModel.Enemy];
      }
}

this method is kind of easy to test, you have only 3 possible scenarios:

  • Case 1: the level doesn't exist,
  • Case 2: the enemy doesn't exist for that level,
  • Case 3: the level and the enemy exist, the difficulty is set to the expected value.

If you create the difficulty matrix from a configuration file you shouldn't test any further and you shouldn't limit the configuration to be what you want. Anyway if you are really looking for limiting the matrix to what you want it to be, you could write a factory class like this:

 internal class DifficultyMatrixFactory
 {
       internal IDictionary<Level<IDictionary<Enemy, Level>> Create() {
          //create the matrix here
       }
 }

and you can create a unit test to ensure that the matrix returned is exactly the one you expect. And this way you prevent other developers to add new difficulties.

ddfra
  • 2,413
  • 14
  • 24
  • Hello, thank you for your response. I actually think that what you are proposing is a brilliant solution. However at this moment in time, this would be too big of a refactor. Also, in this particular case it would work, but we also have a lot of switch cases where specific formules are being selected instead of just an Enum. I could think of ways to make this work, but at this moment in time, this is not what we want. Also, I made a mistake by making this protected method internal. Hence, the method showed above is not one to be tested. I also saw the 'big switches' are mostly protected. – Woudjee Sep 21 '21 at 06:26