We TDD'd; Now we Transform
TDD is great. I love it. It doesn't get the code into the state we want it. it provides a safety net that ensures that everything we've implemented continues to function.
We can still refactor the code. These are refactoring practices that don't always fit during the TDD cycle. If our refactor requires significant changes to the tests... I like to hold those until after the requirements are implemented.
During this transformation, I'm likely to do slightly larger commits. I'll walk through my thinking in text, but just commit the final result.
Transform FizzBuzz
I have a couple MAJOR practices that I like to refactor my code to align with.
No Getters (or Setters)
This one change will simplify about 70-90% of your code, depending on what it looks like before. :)
An incorrect way to look at "No Getters" is to drop the "get" prefix. No. Just... No.
A simple way to look at doing "No Getters" is to not return a class variable. This helps. As we then have to move whatever code is operating on the class variable into the class itself. This is what we want to happen, but it's better to explain what that is.
"No Getters" is about moving the behavior on a piece of data into the class that holds the data. Move the behavior to the data.
I've got some other blog posts on this practice No Getters (or Setters) for more info. We'll cover it more as we encounter it.
No Primitives
One that's definitely going to be a driver for our transformation of FizzBuzz - No Primitives.
Primitives in code are the base data types, int, double, string. It's also collections, List, Map, Dictionary...
I don't think collections are going to show up here, but we definitely have some primitives. We don't want primitives. They are a big No No. This is going to be our initial ... I think... driver on refactoring the code.
Code complexity goes up as primitives move around the system. When we allow behavior to be anywhere, we lose cohesion and can re-implement the same behavior in many places, sometimes incorrectly.
Primitives also hide context. What is 12.7? Is it a distance? A temperature? Money? We don't know. There's no context with it.
public double Convert(double val){...}
What's this method do? Without A LOT more detail, or reading the code, I have no idea. If we have to read the code to figure out what a method does, the method signature is vastly lacking in information.
Let's say that it converts from from temperature scale to another. Which one? Which way? How do I know if I can use this with -40.0?
When we avoid primitives, a signature like this becomes
public Celcius Convert(Kelvin val){...}
And we know exactly what it does. It converts Kelvin to Celcius.
Now, this will only take a Kelvin input and return a Celcius result. The result will only be usable in another calculation using Celcius.
Using double as the return type, I could multiply the result by itself. What is that? It's nonsense. What's a temperature squared?
We're able to do meaningless operations on the raw data because there's no context around it that limits the behaviors available.
When we constrain the raw data to an object that represents the concept, we also constrain the available behaviors. We could expose addition and subtraction operations, but prevent other mathematical operations that are valid on a double, but invalid on a temperature. We could prevent subtracting beyond absolute zero. That's not gonna happen without something that prevents access to the data itself.
Interface for Behavior Contracts
Interface here doesn't always mean the keyword interface. We should have a behavior contract for our objects that allows us to decouple our code from the contract type. It's the "program to the interface" principle we should have. As long as we have an interface, we're good.
Since this is in C#, I'm pretty "Interface Everything" which ensures we're about to avoid coupling to any concrete implementation.
The big ones
Those are the big three I think should be highlighted before we start. I'm sure some others will come up, and I'll talk about them as we encounter them. If you want to see all of the practices, and my posts about them; go here
Refactor!
It's time to refactor! There's going to be things I do that may not make a lot of sense right away. I'll try to explain why, but some of it does come down to ... It's what I've been doing for a long time and it's just how I go about it.
I never like that answer, so I'll try to explain more.
Let's look at our method
public string Transform(int source)
{
if (0 == source % (3 * 5)) return "FizzBuzz";
if (0 == source % 5) return "Buzz";
if (0 == source % 3) return "Fizz";
return source.ToString();
}
Starting off, we have multiple obvious issues of string being around. Not just as a return type, but some hard coded values just floating there.
Out input is an int. We have a few ints being used in the method as well.
Clearly SOMEWHERE has to have the raw data - Absolutely. We just want to limit the classes that touch those primitives. Some things will operate on primitives without holding them; we'll get to those.
I could probably start in any number of places. I like to find what will require the least amount of changes over all. Do a small surgical strike to the code.
Lots of small surgical strikes.
Keep the tests passing and keep modifying the code.
What I see here as a simple one - The hard coded strings. We'll eventually also have to transform source.ToString, but we can put that off since it's an int to string, dealing with two primitives.
This approach to transforming the structure of the code mirrors the "Minimizes Elements" of the 4 rules of simple design. We're not "minimizing elements" but we're "minimizing elements" impacted.
Transform what to what?
What are these? What's the concept we're trying to represent in the code? My guiding principle in development is to
Have a representation for every concept that exist in the code.
What's the concept being represented?
A FizzBuzz Answer. If we're looking at this as the game, it's the answer. We could also see it as a FIzzBuzz Transformed but ... doesn't feel right. Fizz Buzz Answer though; that seems like a pretty close concept.
Somehow we're going to have to encapsulate our string values into something that is a FizzBuzzAnswer.
How do we do that? Well...
How to Transform
How do we transform from a string to an object... to a string?
public string Transform(int source)
{
if (0 == source % (3 * 5)) return "FizzBuzz";
if (0 == source % 5) return "Buzz";
if (0 == source % 3) return "Fizz";
return source.ToString();
}
We want to encapsulate the string "FizzBuzz" in an object. Which seems a bit dumb since it's instantly a string again to return out of the method. Which... Yes, absolutely.
We need to encapsulate all of our current strings, and source.ToString(), in objects that all implement the same type so that we can refactor that signature to the shared type. Given our code concept, this type will be some form of FizzBuzzAnswer.
Why not now?
I don't like to make that change now, because the code won't run until i've fixed it all. I have to update the method, all return points, and the tests, and figure out a way to compare objects... there's A LOT to be done if we try to do it all.
Just One
I like to modify just one of the return points at a time. Using one of the hard coded strings is the easiest. I have a pattern for how I do this. It's been developed over many years and is pretty C# focused, though I've done variations in other languages.
The evolution of how I encapsulate primitive types is described in this blog post . I don't want to rehash that. Go read it if you care.
... You should probably take the 5 minutes and read it even if you don't.
Now that you've read it, or don't care about it; the result is this abstract class.
[DebuggerDisplay("{AsSystemType()}")]
public abstract class ToSystemType<TSystemType>
{
public static implicit operator TSystemType(ToSystemType<TSystemType> origin) => origin.AsSystemType();
public abstract TSystemType AsSystemType();
}
And with a bunch of documentation it looks like
/// <summary>
/// This class allows automatic conversion to system/primitive types at the edge of our system.
/// This should be used with a Marker Class (see https://en.wikipedia.org/wiki/Marker_interface_pattern except a class not interface). The marker class serves to Represent the Concept in the code.
///
/// The values this provides are:
/// * Enables a strict approach approach to "No Primitives".
/// * Allows the edges of the system ignorant conversion to the externally required types.
/// Types become owners of their conversions by implementation of the abstract <see cref="AsSystemType"/>.
/// </summary>
/// <typeparam name="T">Type to auto convert to</typeparam>
[DebuggerDisplay("{AsSystemType()}")]
public abstract class ToSystemType<T>
{
/// <summary>
/// The implicit operator to the generic type allows automatic conversion
/// to system/primitive types at the edge of our system.
///
/// </summary>
/// <param name="origin">The object to get the <typeparamref name="T"/> value from.</param>
public static implicit operator T(ToSystemType<T> origin) => origin.AsSystemType();
/// <summary>
/// Retrieves the value of the type from the concrete class explicitly.
/// </summary>
/// <returns>The converted value of the implementing type.</returns>
protected abstract T AsSystemType();
}
This is my current iteration of how I do encapsulation of smoothly translating an object into a system type. I use SystemType instead of primitive because this is also used when I abstract 3rd party code. If I want to wrap a HttpClient I can use this and pass the object to 3rd party code that expect an HttpClient.
This being an abstract class does mean that implementers can only have one system type transformation. I've never needed more. In situations where I've needed something similar, I've been able to have a type that will transform into the desired type which is represented as a behavior that returns a new object. if I want a temperature as a string (assuming the default is double), I'll have a AsDisplaymethod which returns a TemperatureDisplay which implements ToSystemType<string>.
I won't argue against this seeming like overkill... I will argue that it's fast, effective, and classes are very single responsibility.
It's worked very well across multiple projects, so I stick with it.
Now that we have a way to convert to a string... this doesn't represent the concept we talked about. We need to have a concept that extends this abstract class. Something like a "FizzBuzzAnswer". Unfortunately for us, there is an answer that is "FizzBuzz". "Answer", or "GameAnswer" are both pretty good candidates. I'm going to go with just Answer. Small projects are harder to get good names in. The context is small, which opens up a lot of options. There's not a lot that the concepts need to be distinguished from.
A previous exercise doing this uses Result. Which is also accurate. I don't like to always name things the same. Especially if there's not a lot of restricting contexts. Using some variety as I do these exercises helps me find names that feel more or less appropriate. Or no change.
To get started, I need to ass the ToSystemType into the project. I'm going to drop it into a lib folder. main 3f5d8d2
Answer...s
Now I can create the marker Answer.
public abstract class Answer : ToSystemType<string>{}
A nice simple marker class. I'm not sure "Marker" is the right name for it. It's a Representation/Concept class.
It doesn't currently do anything, and that's OK. That's typical of the Representation classes. (I'm gonna try out this terminology.)
Once we get a few implementations of this, we might see some shared behavior pulled into this class.
As a simple first Answer implementation, let's keep looking at "FizzBuzz". My default naming scheme for a class that derives from a Representation Class is to suffix the abstract name to whatever the specific implementation is. In this case it'll become FizzBuzzAnswer. It's pretty descriptive of what the "FizzBuzz" string represents. That's exactly what we're using this for. We're giving a descriptive, and explicit, name to the concept. We can now talk about the "FizzBuzz Answer". It's a thing we have.
Creating the class and inheriting gives
public sealed class FizzBuzzAnswer : Answer
{
protected override string AsSystemType()
{
throw new NotImplementedException();
}
}
Since we're dealing with a hard coded value, we can return it directly from the AsSystemType method.
I've changed the AsSystemType from public to protected as I don't like the method being public. I prefer to see hard casts making the code smelly. We shouldn't be doing it, so when we do, it should smell. main 21c43af I just hope this doesn't bite me in the ass later.
Our class now looks like
public sealed class FizzBuzzAnswer : Answer
{
protected override string AsSystemType() => "FizzBuzz";
}
Simple. main adcdad8
Let's use it! main 30deede
public string Transform(int source)
{
if (0 == source % (3 * 5)) return new FizzBuzzAnswer();
if (0 == source % 5) return "Buzz";
if (0 == source % 3) return "Fizz";
return source.ToString();
}
And everything's all happy. Tests are still green.
I've discovered recently that Visual Studio and/or ReSharper add a little icon when there's an implicit cast happening. As seen here.
it's new and shiny to me, and wanted to highlight that the IDE is helping us understand what our code is doing.
Since we're at a "did a thing" point, we should refactor the code. At the moment, that'll typically result in moving classes out of our test file.
When I have very small classes implementing the same Representation Class, I like to keep them in the Representation Class file.
public abstract class Answer : ToSystemType<string>{ }
public sealed class FizzBuzzAnswer : Answer
{
protected override string AsSystemType() => "FizzBuzz";
}
In some languages, like Kotlin, I could prevent any classes out side of this file from extending Answer. It's nice to have when I want it.
Now Fizz
Let's do the same thing for our Fizz Answer.
It's a pretty simple copy/paste and modify to create the new class FizzAnswer. main 1b46ad6
And using it is just like FizzBuzzAnswer. main 5c3acaf
public string Transform(int source)
{
if (0 == source % (3 * 5)) return new FizzBuzzAnswer();
if (0 == source % 5) return "Buzz";
if (0 == source % 3) return new FizzAnswer();
return source.ToString();
}
Let's repeat for Buzz next. We'll put off source.ToString for as long as possible.
public sealed class BuzzAnswer : Answer
{
protected override string AsSystemType() => "Buzz";
}
Our implementation is dang familiar main 52ce474
And use is fast and easy main cb4d7c9.
public string Transform(int source)
{
if (0 == source % (3 * 5)) return new FizzBuzzAnswer();
if (0 == source % 5) return new BuzzAnswer();
if (0 == source % 3) return new FizzAnswer();
return source.ToString();
}
This is now instantiating and returning a type, not using it. The fact they are getting converted to strings... well... We'll fix that. In the mean time, it's not us doing the conversion in the method. I don't really like that excuse because it's a slippery slope to abuse of the implicit casts.
Let's just focus on the fact we'll resolve it later.
The conversion of a source.ToString to an Answer may not be as hard as I thinking. It'll need some future updates... but for now we can create an StringOfAnswer class that encapsulates the knowledge of how we convert the int to a string. The game is to give back the input as the answer, and we need it as the String of that input. Maybe we should go for StringOfInputAnswer or InputAsStringAnswer. I kinda like this one. Input As String gives us a pretty clean read of what's going to happen.
public sealed class InputAsStringAnswer : Answer
{
private readonly int _input;
public InputAsStringAnswer(int input) => _input = input;
protected override string AsSystemType() => _input.ToString();
}
A smell when creating this class is that it takes an int. That's not a huge problem, but taking in primitives is something to keep an eye on. Something has to, so it's not bad, but that means something else has to provide an primitive, and that's where the concern is. Parameters of the primitive type force other classes to pass around raw data. We don't want that.
We also know that we want to change the parameter of the method away from an int... so this method is likely to have to change. That's OK, we're going step by step and staying Green.
Using this new class has our Transform method looking like this main 1871728.
public string Transform(int source)
{
if (0 == source % (3 * 5)) return new FizzBuzzAnswer();
if (0 == source % 5) return new BuzzAnswer();
if (0 == source % 3) return new FizzAnswer();
return new InputAsStringAnswer(source);
}
You'll notice that we're no longer invoking ToString here. How we change to the string form is decoupled from this method. It no longer needs to know how to transform. There's a class, InputAsStringAnswer that has that responsibility.
Representation shifts Responsibility
By creating objects to represent the concepts we've been able to shift the responsibility of knowing how to generate things. Hardcoded strings are simple generations. Using ToString is simple. What if there's a database, or network call - By decoupling the source of the raw data of the answer (eg Fizz) from where we decide what answer to provide (Transform method) we're making the system more flexible and less complex.
We are making each individual piece of the code have a narrower scope of change. Previously, if any of the values of the answer changed, we'd have to modify Transform. That provided 4 different reasons to change.
- FizzBuzz -> FooBarr
- Fizz ->Fyz
- Buzz -> BeeSound
- source.toString() -> source.ToString("D8") //Keeps everything aligned
That's just the answers. I we want to vary the conditions, even more reasons to change.
By extracting each answer to a class, that's the only place that has to change. Each Answer class has become /the/ authoritative answer for it's concept. It's the S.PO.T. Single Point Of Truth. No where else should ever decide what the FizzBuzzAnswer value is. We have a single place in the entire system where that's defined, controlled, and, when required, modified.
Return Type
Now that we've encapsulated all of our answers in an extension of Answer we can change the return type of Transform. main c3f3d06
public Answer Transform(int source)
{
if (0 == source % (3 * 5)) return new FizzBuzzAnswer();
if (0 == source % 5) return new BuzzAnswer();
if (0 == source % 3) return new FizzAnswer();
return new InputAsStringAnswer(source);
}
And we stay Green!
With the implicit conversion to a string that allowed us to wait to change the return type will change to a string in the tests. We don't have to change those.
We can. Now that the tests get an Answer back, we could go update them and... Add comparisons to Answer? Sure. I think that's reasonable... but also... not while we're trying to get rid of primitives.
If I'm able to keep the tests unmodified while refactoring the system, all the better. When I don't have to change the tests, I can have absolute confidence that the behavior still functions. For me, confidence goes down slightly as tests get modified. I can double check them and see they still function. That's me restoring the confidence. That I feel the desire to do so means I've lost confidence in them.
No Primitive... string
We've gotten rid of the string primitive from Transform. string is still hanging out a little in the tests, and the ToSystemType abstract methods, but, that's it. We've removed string from anywhere the raw data is not needed.
We can still see some int type and values.
Next Time
We've created our Answer classes in this post. I think it's a good point to take a break and we'll approach how to transform our input into an object and see how that change cascades through the system.
Remember, when we have this line if (0 == source % (3 * 5)) return new FizzBuzzAnswer(); and source is an object... how do we get the int value out to do the mod on? We can't. That's a huge part of the object!
What do we do?
We'll see next time.