Is DRY good or can it break O from SOLID

The principle of DRY (Do not Repeat Yourself) has long been obvious to everyone and is loved by many programmers. And many agree that Copy / Paste is not cool at all. In this article I want to give an example of when in industrial programming the use of Copy / Paste is more appropriate and helps to beautifully implement the Open-Closed principle from SOLID.

Let me remind you that the Open-closed principle encourages programmers to design classes so that they are open for extension, but at the same time closed for modification. Modifying a class is allowed only if an error is detected in the class. If you want to add functionality, then the principle calls for creating a new class and using either inheritance or implementation of the same interface.

For example, there is a parcel management system. Suppose, the task has come to add the ability to create, in addition to simple packages, also urgent ones.

We have a Parcel class that describes how a regular package works:

public interface IParcel {
      string Barcode {get; set;}
}
public class Parcel: IParcel  { 
      public string Barcode {get; set;}
}

It is very tempting to simply add a field to both the old Parcel class and the IParcel interface:

public interface IParcel {
      string Barcode  {get; set;}
      bool   IsUrgent {get; set;}
}
public class Parcel: IParcel  { 
      public string Barcode  {get; set;}
      public bool   IsUrgent {get; set;}
}

However, such code should not pass CodeReview! A strict and experienced code checker should return it with the remark: “such an implementation violates the Open-closed principle.”

It is much better to create a new UrgentParcel class, and you will not need to change the interface or the Parcel class. Class and interface files will remain untouched:

public class UrgentParcel: IParcel { 
      public string Barcode {get; set;}
}

This will be an observance of the Open-closed principle, and such code will not receive comments with CodeReview.

Now let's get back to DRY and the way in which it makes it difficult to implement the Open-closed principle.

Imagine that in the Parcel class we have the “status of the package” field and some logic for changing this status:

public class Parcel: IParcel  { 
      public string Barcode {get; set;}
      // Статус посылки (в пути, или уже доставлена и т.п.)
      public ParcelStatuses Status {get; }
      // метод, который меняет статус посылки на "доставлена"
      public void ArrivedToRecipient(){
            this.Status = ParcelStatuses.Arrived;
      }
}

Does this logic need to be copied to the UrgentParcel class? The DRY principle says that by no means. It is much better that the UrgentParcel class simply inherits from the Parcel class, which solves the problem and does not have to Copy / Paste the body of the ArrivedToRecipient method to the UrgentParcel class.

However, if you do not copy the code, but inherit it, then changes to the ArrivedToRecipient method in the Parcel class will immediately lead to a change in the behavior of the UrgentParcel class, which will violate the Open-closed principle. This is really a violation, because the task with urgent parcels (implementation of the UrgentParcel class) has already been delivered, tested and, as a result, works “in battle”. So, this logic, implemented in the UrgentParcel.ArrivedToRecipient method and applied to urgent parcels, suits everyone and it should NOT change when the work of other types of parcels changes. So, the Open-closed principle is precisely designed to protect the system from such actions by inexperienced junior programmers who, while solving the problem, as usual “head on”, do not yet realize all the dependencies

Usually, one of the main arguments in favor of DRY is the fact that if an error is found in the ArrivedToRecipient method, then it should be fixed wherever it was copied. So this just does not need to be done. If an error is found in the ArrivedToRecipient method when working with regular packages, then it is necessary to correct the work of ordinary packages. But nobody complained about the work of urgent parcels and, probably, everyone is happy with how urgent parcels work.

For perfectionists, to whom I also consider myself, I would suggest leaving a comment that would allow us not to forget about all the places where the method was copied, and help raise the question: does this method work correctly for urgent parcels?

Here is an example of such a comment:

public class Parcel: IParcel{ 
    ... 
    /// 
    /// Проставляет посылке статус "доставлена" 
    ///     
    /// NOTE: код метода копировался в классы: 
    public void ArrivedToRecipient(){ ... }
}
public class UrgentParcel: IParcel{ 
    ... 
    /// 
    /// Проставляет посылке статус "доставлена"
    ///     
    /// NOTE: код метода был скопирован из класса: 
    public void ArrivedToRecipient(){ ... }
}

Very interesting is the opinion of the community on this issue. Thanks in advance for your comments.

Also popular now: