How to refactor a method which breaks "The law of Demeter" principle?

Posted by dreza on Programmers See other posts from Programmers or by dreza
Published on 2012-06-29T22:25:43Z Indexed on 2012/06/30 3:23 UTC
Read the original article Hit count: 196

Filed under:
|
|

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?

© Programmers or respective owner

Related posts about c#

Related posts about design