Abstract
I have been trying to get my head around the cohesiveness of some functionality in our code base. I’ve approached this design in different ways, and lately I’m convinced that I took the wrong approach, as in I incorrectly applied the Single Responsibility Principle.
Issue
The problem domain is a Well, otherwise known as a round hole in the ground. The current code looks something like this... I have a set of data in a data only object...
Note: This is not the full code, just method names to keep things short...
public class WellData
{
public string Name { get; set; }
public ICollection<SurveyPoint> SurveyPoints { get; set; }
public ICollection<GeometryItem> GeometryItems { get; set; }
public ICollection<TemperaturePoint> WellTemperature { get; set; }
public ICollection<FluidPoint> WellFluids { get; set; }
}
I broke functionality that operates on this data into several small classes. As per below…
public class ReferenceWellSurvey
{
ICollection<SurveyPointCalculate> _calculatedSurveyPoints;
double GetTvdAtDepth(double depth) {}
double GetAzimuthAtDepthRad(double depth) {}
double GetInclinationAtDepthRad(double depth){}
double GetTortuosityPeriodAtDepth(double depth ) {}
double GetTortuosityAmplitudeAtDepth(double depth ) {}
}
public class ReferenceWellGeometry
{
ICollection<GeometryItem> _geometryItems;
double GetFrictionAtDepth(double depth) {}
double GetHoleIdAtDepth(double depth) {}
}
public class ReferenceWellTemperature
{
ICollection<TemperaturePoint> _wellTemperatures;
double GetTemperatureAtDepth(double depth) {}
double GetSurfaceTemperature(double depth) {}
}
Etc.
I’ve realized lately when looking at our code that I have to glue all this back together to make it usable where I need it in the application logic. Something like….
public class ReferenceWellData
{
private IReferenceWellGeometry _refWellGeometry;
private IReferenceWellTemperature _refWellGeometry;
private IReferenceWell _refWellGeometry;
private IReferenceFluid _refWellFluid;
//Proxy functions from each class to glue it all back together…
}
A few things that led me to the current mess. The well data class contains several collections of information and if everything is in one class most functions will only operate on a small subset of the data. At the time I felt that would create low cohesion, now a few months later, I feel these things do actually all belong together. Additionally, there are several classes in my code that need to call just one method of the ReferenceWellSurvey class, GetTvdAtDepth(). Really what I did didn’t solve this issue.
After thinking more about this, and after reading this Wikipedia article http://en.wikipedia.org/wiki/Cohesion_(computer_science), I’m starting to see that I have committed a few programming sins.
• Low cohesion
• Constructor over injection in code where I try to glue things together
• Giving a class that depends on GetTvdAtDepth access to other methods that it does not need, violated Interface Segregation Principle.
Solution
I’m looking for that Goldilox design that doesn’t give too much responsibility to any one class (No God Objects), while yet maintaining a good degree of cohesion.
My current thoughts on fixing this are as follows. Move the functionality that operates on the well data into the same class. This should help get this functionality closer to Communicational/informational cohesion.
public class WellData : IWellData
{
public string Name { get; set; }
public ICollection<SurveyPoint> SurveyPoints { get; set; }
public ICollection<GeometryItem> GeometryItems { get; set; }
public ICollection<TemperaturePoint> WellTemperature { get; set; }
public ICollection<FluidPoint> WellFluids { get; set; }
// Operates on SurveyPoint collection
double GetTvdAtDepth(double depth) {}
double GetAzimuthAtDepthRad(double depth) {}
double GetInclinationAtDepthRad(double depth){}
double GetTortuosityPeriodAtDepth(double depth ) {}
double GetTortuosityAmplitudeAtDepth(double depth ) {}
//Operates on WellGeometry collection
double GetFrictionAtDepth(double depth) {}
double GetHoleIdAtDepth(double depth) {}
//Operates on WellTemperature collection
double GetTemperatureAtDepth(double depth) {}
double GetSurfaceTemperature(double depth) {}
}
Next to address the issue of several classes needing just one function, I could make this one function into a separate interface IGetTvdAtDepth. In this way, my classes that need that functionality can depend just on this interface, which I can inherit into my WellData interface, and implement it there, but not tie my other classes to all the functionality that is in my WellData class.
Thoughts on this approach vs. the original approach? One thing that bothers me about doing things way is that in my API I'm using bastard injection to fulfill the required dependencies of this class... Right now I have three required dependencies, a Survey calculator, and a special class that references data at different measured depths. I'd like to move away from this, but that seems to point back toward having services do more of the lifting...
RefWellData.GetFluids().AtDepth(100)
orRefWellData.GetTemperatures().AtSurface()
? To me, this looks like a simple composition exercise; is there something that makes such an approach inconvenient?