How to refactor a method which breaks "The law of Demeter" principle?
- by dreza
I often find myself breaking this principle (not intentially, just through bad design). However recently I've seen a bit of code that I'm not sure of the best approach.
I have a number of classes. For simplicity I've taken out the bulk of the classes methods etc
public class Paddock
{
public SoilType Soil { get; private set; }
// a whole bunch of other properties around paddock information
}
public class SoilType
{
public SoilDrainageType Drainage { get; private set; }
// a whole bunch of other properties around soil types
}
public class SoilDrainageType
{
// a whole bunch of public properties that expose soil drainage values
public double GetProportionOfDrainage(SoilType soil, double blockRatio)
{
// This method does a number of calculations using public properties
// exposed off SoilType as well as the blockRatio value in some conditions
}
}
In the code I have seen in a number of places calls like so
paddock.Soil.Drainage.GetProportionOfDrainage(paddock.Soil, paddock.GetBlockRatio());
or within the block object itself in places it's
Soil.Drainage.GetProportionOfDrainage(this.Soil, this.GetBlockRatio());
Upon reading this seems to break "The Law of Demeter" in that I'm chaining together these properties to access the method I want. So my thought in order to adjust this was to create public methods on SoilType and Paddock that contains wrappers i.e.
on paddock it would be
public class Paddock
{
public double GetProportionOfDrainage()
{
return Soil.GetProportionOfDrainage(this.GetBlockRatio());
}
}
on the SoilType it would be
public class SoilType
{
public double GetProportionOfDrainage(double blockRatio)
{
return Drainage.GetProportionOfDrainage(this, blockRatio);
}
}
so now calls where it used would be simply
// used outside of paddock class where we can access instances of Paddock
paddock.GetProportionofDrainage()
or
this.GetProportionOfDrainage(); // if used within Paddock class
This seemed like a nice alternative. However now I have a concern over how would I enforce this usage and stop anyone else from writing code such as
paddock.Soil.Drainage.GetProportionOfDrainage(paddock.Soil, paddock.GetBlockRatio());
rather than just
paddock.GetProportionOfDrainage();
I need the properties to remain public at this stage as they are too ingrained in usage throughout the code block. However I don't really want a mixture of accessing the method on DrainageType directly as that seems to defeat the purpose altogether.
What would be the appropiate design approach in this situation? I can provide more information as required to better help in answers.
Is my thoughts on refactoring this even appropiate or should is it best to leave it as is and use the property chaining to access the method as and when required?
