Search code examples
language-agnostictestingsingletonencapsulationtradeoff

Doesn't Passing in Parameters that Should Be Known Implicitly Violate Encapsulation?


I often hear around here from test driven development people that having a function get large amounts of information implicitly is a bad thing. I can see were this would be bad from a testing perspective, but isn't it sometimes necessary from an encapsulation perspective? The following question comes to mind:

Is using Random and OrderBy a good shuffle algorithm?

Basically, someone wanted to create a function in C# to randomly shuffle an array. Several people told him that the random number generator should be passed in as a parameter. This seems like an egregious violation of encapsulation to me, even if it does make testing easier. Isn't the fact that an array shuffling algorithm requires any state at all other than the array it's shuffling an implementation detail that the caller should not have to care about? Wouldn't the correct place to get this information be implicitly, possibly from a thread-local singleton?


Solution

  • Yes, that does break encapsulation. As with most software design decisions, this is a trade-off between two opposing forces. If you encapsulate the RNG then you make it difficult to change for a unit test. If you make it a parameter then you make it easy for a user to change the RNG (and potentially get it wrong).

    My personal preference is to make it easy to test, then provide a default implementation (a default constructor that creates its own RNG, in this particular case) and good documentation for the end user. Adding a method with the signature

    public static IEnumerable<T> Shuffle<T>(this IEnumerable<T> source)
    

    that creates a Random using the current system time as its seed would take care of most normal use cases of this method. The original method

    public static IEnumerable<T> Shuffle<T>(this IEnumerable<T> source, Random rng)
    

    could be used for testing (pass in a Random object with a known seed) and also in those rare cases where a user decides they need a cryptographically secure RNG. The one-parameter implementation should call this method.