There is no I in team and there is no WHY in patterns or practices. But there should be. Too many people abuse patterns because they didn't learn the WHY.
Accepting and using a coding pattern/practice without knowing what problem it tries to solve will make your code a mess. This is one of my favorite pet peeves when it comes to coding. And I guess I can be quite annoying about it because I like to call my colleges out on it. When they ask me to do a refactoring I don’t understand the benefit of I will ask:
- What is the benefit?
Too often the answer is simply “It is good practice to do [Fill in some overused practice here]”. If you get this answer or something like it there should be alarms ringing in your head. Had the other person actually understood WHY, they would have told you that instead. It’s a big difference of understanding not only how but also why.
One of the most common situations this comes up is when someone asks me to reduce the number of parameters to a method by wrapping several of the arguments in some form of context object. Usually they have read Clean Code or something similar and think that having a large number of arguments makes children cry and kittens die. But are a large number of arguments really the problem? Isn’t it just a symptom?
To be clear I DO think that many arguments suggest that something is wrong but only because it suggests that the method is too complex and should be refactored if possible. IMO It is the complexity of the code and NOT the number of arguments that is the real problem.
As an example look at the original code here:
/// <summary>
/// Calculates the distance between two geo-points in kilometers using the Haversine algorithm.
/// </summary>
/// <returns>A double indicating the distance between the points in KM.</returns>
public static double Haversine(double long0, double long1, double lat0, double lat1)
{
double dLat = ToRad(lat1 - lat0);
double dLon = ToRad(long1 - long0);
double a = Math.Pow(Math.Sin(dLat / 2), 2) +
Math.Cos(ToRad(lat0)) * Math.Cos(ToRad(lat1)) *
Math.Pow(Math.Sin(dLon / 2), 2);
double c = 2 * Math.Atan2(Math.Sqrt(a), Math.Sqrt(1 - a));
double distance = EARTH_RADIUS_KM * c;
return distance;
}
And then at the modified code:
public class HaversineContext
{
public Double Long0 { get; set; }
public Double Long1 { get; set; }
public Double Lat0 { get; set; }
public Double Lat1 { get; set; }
}
/// <summary>
/// Calculates the distance between two geo-points in kilometers using the Haversine algorithm.
/// </summary>
/// <returns>A double indicating the distance between the points in KM.</returns>
public static double Haversine(HaversineContext context)
{
double dLat = ToRad(context.Lat1 - context.Lat0);
double dLon = ToRad(context.Long1 - context.Long0);
double a = Math.Pow(Math.Sin(dLat / 2), 2) +
Math.Cos(ToRad(context.Lat0)) * Math.Cos(ToRad(context.Lat1)) *
Math.Pow(Math.Sin(dLon / 2), 2);
double c = 2 * Math.Atan2(Math.Sqrt(a), Math.Sqrt(1 - a));
double distance = EARTH_RADIUS_KM * c;
return distance;
}
I have reduced the number of arguments but made my code harder to read and I haven’t reduced any complexity. I’m still dependent of four variables (+ the context variable too). What I find event worse is that it is less clear what the Haversine method expects. Do I have to set all the properties or is it only using two of them? I have simply shuffled the problem around, but hey I only got one argument now so I must be awesome, right? If you only have a shallow understanding of the concept it might look better while it is worse for you code.
The conclusion is that you should understand WHY. Until you do you don’t understand anything. And when other people try to force their practices on you ask them why. If they know why you might learn something and if they don’t you need to stand up for your code to avoid a mess of abstractions.