Replace bool?[] with a custom type of Line

[mmmcaffeine]

Overview

Trying to represent all cells with an IEnumerable<bool?> which we internally convert into bool?[] is clumsy. The true, false, or null has meaning in our domain which is implicit. We can be more explicit about this. Setting up data is also clumsy and can be seen by test cases being awkward to visually parse e.g. what was the index of that true value again? When debugging issues with IStrategy implementations it was difficult to see what the data genuinely was.

We should introduce a custom type of Line. This will be an IEnumerable<CellState> where CellState is an enumeration where we explicitly state meaning.

Required Behaviour

• Construct with an IEnumerable<CellState> that contains at least one item
• Implicit conversion to and from bool?[]
• Static Parse method that accepts a string of e.g. "001_1_00"
• Static TryParse method following the accepted convention
• Comparison operators to string
• Support enumeration
• Support indexing
• Be immutable (which means not exposing e.g. CellState[] because it would allow people to change state, although you could expose e.g. IReadOnlyCollection<CellState>

[mmmcaffeine]

Because we have implemented implicit conversion to string we get comparison operators for free. I think this is the better approach. We would not get the implicit conversion for free if we only implemented the equality operators.

[mmmcaffeine]

We have implemented an implicit conversion to and from bool?[]. However Array is a reference type so it is possible to pass a null. Even though we have nullable types enabled it is trivial to "defeat" the compiler with the null forgiving operator. At the moment that is going to cause a NullReferenceException as we attempt to enumerate the null.

It is unclear what the best approach here is. If we throw an ArgumentNullException we cannot have this declared as an implicit conversion which reduces its usefulness. We could return some form of empty Line i.e. one with an empty CellState[]. However, we don't currently expose the array so the default of Equals would not give the required behaviour.

[mmmcaffeine]

We started implementing this as a record struct. This was largely because we considered the type as immutable, and we wanted to pick up a lot of behaviour for free. However, it seems we're moving away from the expected behaviour of a record:

• We're not using positional parameters
  • Default implementation of Equals doesn't work well for us
  • Default implementation of ToString doesn't work well for us
  • here is no use for with expressions
• Deconstruction doesn't make sense
• We don't (directly) expose the data it is constructed with

As such it might be better to implement this as readonly struct. We do tick a lot of the boxes in the Microsoft recommendations. The biggest sticking point is the use of an array internally. That means we can't guarantee the size of the object.

In light of both record and struct having issues with the usage we might also stick with the safe option, implement as a class and make the type immutable through careful implementation. This would, however, make life harder when implementing implicit conversions from Line; we can no longer rely on the input parameter not being null.

[mmmcaffeine]

Initially the plan was to implement CellState as an enumeration. It became apparent this was probably not correct. There were a number of places in Line we were switching on the enum. There were also a number of places we had knowledge of what were considered valid characters. A lot of this could be improved by treating CellState as a record with public static properties to expose the specific states we are modelling.