Let us now continue to refactor according to good design principles.
Let us consider this principle:
- Depend upon abstractions and not concrete implementations.
What does this mean?
Consider this code:
public class WarDepartment
{
public WarDepartment()
{
Army alliedFores = new Army();
alliedFores.Fight();
}
}The War Department is entirely reliant upon the Army class. Without the army class, then the War Department is entirely useless. Moreover the war department is stuck with the army. If you want a war department, then you’re gonna lug along an army as well. Imagine if you want a suitcase, but in order to use the suit case you also have to lug around your grand piano. They are joined at the hip. Such is the nature of this particular dependency.
Let’s change this around a little bit:
public class WarDepartment
{
Army alliedFores;
public WarDepartment()
{
this.alliedFores = new Army();
}
public void ProsecuteWar()
{
alliedFores.Fight();
}
}Now the war department has a little bit more control. In the first instance above, you couldn’t control when you could prosecute the war. Now you can. You have a little bit more control. And you’ll notice that all of the methods make sure of the Army instance. This class is highly cohesive, but the problem is that you’re limited to an army.
But what if you want an airforce?
Now if you want an Airforce you could pass in a flag:
public class WarDepartment
{
Army alliedFores;
Airforce airforce;
public WarDepartment(bool army)
{
if (army)
{
this.alliedFores = new Army();
}
else
{
this.airforce = new Airforce();
}
}
public void ProsecuteWar()
{
if (army)
{
alliedFores.Fight();
}
else
{
airforce.Fight();
}
}
}But this is bad practice and is a very smelly code smell. A better approach would be to inject a common interface between the army and the airforce. Let’s create one and call it the armed forces:
public class WarDepartment
{
ArmedForces union;
public WarDepartment(ArmedForces union)
{
this.union = union;
}
public void ProsecuteWar()
{
union.Fight();
}
}
public interface ArmedForces
{
void Fight();
}
public class Army : ArmedForces
{
public void Fight()
{
Console.WriteLine("Shouting the battle cry of Freeeeeedom!");
}
}
public class Air : ArmedForces
{
public void Fight()
{
Console.WriteLine("The Union forever, hurrah! boys, hurrah! ");
}
}
public class Navy : ArmedForces
{
public void Fight()
{
Console.WriteLine("Down with the traitor, and up with the Star!");
}
}Now the War department has some degree of flexibility. It can pass in any concrete type of armed forces. You still need a force to prosecute a war. But you have some degree of flexibility. (Side note: Indeed it was this flexibility and tenacity that allowed General Grant to successful prosecute the Vicksburg campaign, that effectively sealed the fate of the Confederacy. Flexibility in options, generally speaking, is a good thing.)
Let us similarly identify the dependencies in our code and see if we can more loosely couple it:
public abstract class NewsPaper
{
public void PrintInfo();
}
public abstract class Printer
{
public void Print();
}
public class AggregateBeamsNewspaper : NewsPaper
{
private Model model;
private ExcelPrinter printer;
private List<BeamInfoRequired> beamInfoCollection;
public AggregateBeamsNewspaper(PartSelector<TSM.Beam> beams, ExcelPrinter printer)
{
this.model = new Model();
this.printer = printer;
if (model.GetConnectionStatus())
{
}
}What are we dependent on in the constructor?
-
We are extremely dependent upon: (i) a part selector class (which is a concrete implementation) – which in turn is dependent upon beams (which is actually a class which we have no control over), and (ii) we are also dependent upon an Excel printer.
-
We own the Excel printer so let’s pass in an interface instead of a concrete implementation.
Now it looks like the BeamsNewspaper is doing less and less. What is it really doing? It is organising the Tekla Beams for collection, and then, once they are organised in a printable form, it then delegates the printing to the Printer class.
public class AggregateBeamsNewspaper : NewsPaper
{
// etc.
public AggregateBeamsNewspaper(PartSelector<TSM.Beam> beams, Printer printer)
{
// etc.
}
public abstract class Printer
{
public abstract void Print(List<BeamInfoRequired> beamInfoCollection, string fileName, string filePath);
}
// That looks much better! public class AggregateBeamsNewspaper : NewsPaper
{
public AggregateBeamsNewspaper(PartSelector<TSM.Beam> beams, Printer printer)
{
this.model = new Model();
this.printer = printer;
if (model.GetConnectionStatus())
{
List<AggregateBeam> beamsGroupedByNamePartAndFinish = groupBeamsByPartNameAndFinish(beams);
this.beamInfoCollection = getBeamInformationCollection(beamsGroupedByNamePartAndFinish);
}
}
}
public class BeamsNewspaper : NewsPaper
{
// etc
public BeamsNewspaper(PartSelector<TSM.Beam> beams, Printer printer)
{
this.model = new Model();
this.printer = printer;
if (model.GetConnectionStatus())
{
this.beamInfoCollection = getBeamInformationCollection(beams.ToList());
}
}Let's abstract out the commonalities in the above. Perhaps it's an exercise that you can undertake? What is the difference between the two constructors? It seems that they both deliver a beamInfoCollection but they do so in different ways. We can push the commonalities to the base class above and override this implementation in the sub-class.
In other words: we want the AggregateBeamsNewspaper constructor to look like this:
public AggregateBeamsNewspaper(PartSelector<TSM.Beam> beams, Printer printer)
{
this.model = new Model();
this.printer = printer;
if (model.GetConnectionStatus())
{
this.beamInfoCollection = getBeamInformationCollection(beams.ToList());
}
}So then we'll have to change our methods around a little bit.
Remember, it is best for us to pass in an interface. So if we take things to the extreme leve, we would have to pass in a wrapper which shares the same interface as the Tekla API Beam class – but only with respect to the properties that we are interested in.
Secondly, the printer is very closely tied with the data structure which is passed into it: the BeamInfoRequired stuct. This is problematic. What if we wanted to print more information then what we are currently printing?
-
We’d basically have to create a different type of printer. (or at least overload an existing printer)
-
We’d have to create a different type of newspaper.
But then how do we easily print different types of beam based information? These classes are too closely tied. Perhaps we can think of a way to better decouple?
Still, I am less than pleased with the naming and the structure. I’ll have to think of something a little better than “NewsPaper”.
Photo by Ross Findon on Unsplash
by