Unit tests and protected methods

protected-padlockWhen working with Object Oriented Programming (OOP) languages we have the possibility to design our code and our classes using encapsulation. C# defines the “protected” accessibility level, which is accessible only from the containing class and from the types that inherits from this class. It is very helpful when you follow the Open-Closed Principle (OCP).

On the other side you might ask yourself how can we test the behavior of the methods if they are protected. And how to do so without breaking the encapsulation of the class.

Testing a protected method

I created a small custom class to expose my way of doing unit tests on protected methods, here is a class to test:

public class MyClass
{
    public int Counter { get; private set; }

    protected void IncrementCounter()
    {
        Counter++;
    }
}

The easiest thing I could do to be able to test my method is to replace the “protected” keyword by “public”. But I will not do that because, in my opinion when you write tests after the production code you should try to avoid updating the code as much as possible even to test it.

One other possibility is to make the method “internal”, this keyword specifies that the method can only be access from the current assembly. And then, for your tests project you can make them visible using the following attribute in the Properties/AssemblyInfo.cs file of your production project:

[assembly: InternalsVisibleTo("MyTestProject")]

This option is helpful when you want to limit the scope of an API defined in an assembly but without losing the ability to add unit tests on the entire project.

But again I will not use this trick to test my method, instead I will expose it using inheritance for my tests by creating a new class, only for testing purposes.

public class MyTestClass : MyClass
{
    public void ExposeIncrementCounter()
    {
         base.IncrementCounter();
    }
}

With this new class I can now test the behavior of the protected method without modifying it. And here is my test:

[TestMethod]
public void Test_MyClass()
{
    var myClass = new MyTestClass();
    myClass.ExposeIncrementCounter();
    myClass.Counter.ShouldBe(1);
}

Note that, once again, I use the Shouldly library to make my assertion.

Testing a call to a protected method

The other day one of my coworker was asking how can he test that a protected method is called when testing a public method. This is a tricky question and it depends on the context of your code. In his case the given method was not part of our project and therefore impossible to modify. He was still able to test the behavior of the method by mocking another class used by this protected method.

I think this experience is interesting because it shows that you can still write automated tests by going one level deeper. You might ask yourself if this is still a unit test. Maybe, maybe not but it is a test for your project.

And this story made me think: “How to test the call to a protected method?”. I created the following class to show you a way to be able to do so.

public class MySecondClass
{
    protected void InnerMethod()
    {
        //a lot of logic and dependencies
    }

    public void DoWork(bool doWork)
    {
        if (doWork)
        {
            InnerMethod();
        }
    }
}

In my case I want to test that, depending on the parameter’s value, the DoWork method calls the InnerMethod method. But without executing the code of the latter because it does too many things and therefore the test will be too long (if you have a similar situation you might have some code smells to fix).

To write my tests, this time I will slightly update the code to be able to do what I want. I am not a big fan of this but I don’t really see another option that will not imply more refactoring.

protected virtual void InnerMethod()
{
    //a lot of logic and dependencies
}

I simply made the method virtual in order to be able to override it. I did not really break the encapsulation, I just gave me more options. Now I can create a new class for testing purposes:

public class MySecondTestClass : MySecondClass
{
    public bool InnerMethodHasBeenCalled { get; private set; }

    protected override void InnerMethod()
    {
        InnerMethodHasBeenCalled = true;
    }
}

This class inherits from the class I want to test and does not modify its behavior regarding the DoWork method. It is a test double using the spy technique. I can now write the two following tests to check the behavior of the code.

[TestMethod]
public void Test_MySecondClass_False()
{
    var myClass = new MySecondTestClass();
    myClass.DoWork(false);
    myClass.IncrementCounterHasBeenCalled.ShouldBe(false);
}

[TestMethod]
public void Test_MySecondClass_True()
{
    var myClass = new MySecondTestClass();
    myClass.DoWork(true);
    myClass.IncrementCounterHasBeenCalled.ShouldBe(true);
}

If you write tests after writing the production code it is still possible to do so without changing too much of the code even with protected methods. If the code has been written using encapsulation there must be a reason for that and therefore the tests should not force you to break everything down.

What about private methods?

Now you might ask yourself, how to do this with private methods. With these kind of methods we cannot use inheritance for tests, so how?

Well you can use Microsoft Fakes to mock the calls to private methods, but in my opinion this is definitely overkill. Our projects should be testable without tools like this.

The other solution is to change the accessibility of the method to make it testable, because you believe that “Tests trump Encapsulation“.

But first, I think that when you want to test a private method, you should ask yourself some questions first. Why is this method private? Does it really needs to be private? Can I extract the logic inside into another class I will be able to test? Can I include its logic in the test of another method?

Using encapsulation should mean something from a design point of view. Writing tests after the code is better than writing no test at all. It also make you ask questions about the code you wrote: “What do I want to test?”.

And I think this is why we saw the arrival of practice such as Test Driven Development (TDD) to force us to ask these questions first in order to help us with the design and encapsulation of the production code. I will not enter in the debate of whether should you practice TDD or not. Or does TDD lead to good design or not. I don’t know enough to answer these questions and I especially think that it depends on the developer.

In this blog post I wanted to show you how to test protected methods without having too much impacts on the code, I hope it will help you. And always ask yourself about the Why when it comes to tests and only after the How.

See you next time!

Primitive type obsession

the-old-vs-the-newAs professional developers, we are asked to automate behaviors by writing high quality code. Our favorite programming languages allow us to do so by providing some primitive types we can manipulate in our program. Yet he can be easy to fall into the trap of primitive type obsession. This code smell occurred when we use primitive data types (string, int, bool, …) to model our domain ideas.

The problem

In one of my previous blog post, I demonstrate how to create custom model validation using ASP .NET MVC using an IP address as an example. I had the following property in my Model:

public string IPAddress { get; set; }

This is clearly a primitive type obsession, because using a string to store an IP address can be dangerous, the compiler will not prevent me from writing the following code:

model.IPAddress = null;
model.IPAddress = "";
model.IPAddress = "foobar";

In C# .NET a string can contain much more information than an IP address, the later is far more specific and should be designed accordingly. I will then use Object Oriented Programming and encapsulation to create a custom IPAddress type.

The refactoring

In the other article I explained that the address follows the IPv4 format which is composed of 4 bytes, then let’s create a class representing this concept.

public class IPAddress
{
    public byte FirstByte { get; set; }
    public byte SecondByte { get; set; }
    public byte ThirdByte { get; set; }
    public byte FourthByte { get; set; }
}

Does it make sense to be able to instantiate an IPAddress object without setting the bytes? Not really but for now it is possible, let’s fix that!

public class IPAddress
{
    public byte FirstByte { get; private set; }
    public byte SecondByte { get; private set; }
    public byte ThirdByte { get; private set; }
    public byte FourthByte { get; private set; }
 
    public IPAddress(byte first, byte second, byte third, byte fourth)
    {
        FirstByte = first;
        SecondByte = second;
        ThirdByte = third;
        FourthByte = fourth;
    }
}

I now have an object that actually represents an IP address that must be instantiated with all the data it requires. And I now have a class where I can add related logic to ease its use.

public class IPAddress
{
    public byte FirstByte { get; private set; }
    public byte SecondByte { get; private set; }
    public byte ThirdByte { get; private set; }
    public byte FourthByte { get; private set; }
 
    public IPAddress(byte first, byte second, byte third, byte fourth)
    {
        FirstByte = first;
        SecondByte = second;
        ThirdByte = third;
        FourthByte = fourth;
    }
 
    public static bool TryParse(string ipAddress, out IPAddress ipaddress)
    {
        // some validation logic & parsing logic
        // ...
    }
 
    public override string ToString()
    {
        return string.Format("{0}.{1}.{2}.{3}", FirstByte, SecondByte, ThirdByte, FourthByte);
    }
}

I switched from primitive type obsession to “domain modeling” by creating a value object. This kind of refactoring is very helpful for a lot of other cases. For example country codes stored in string while they are only 2 or 3 characters long with only a pre-defined number of possible values. Or an amount represented by an integer which can be negative and completely leaves out the currency of this amount. These are only a few examples among many.

It’s easy to fall into the trap of this code smell, using primitive data types is quick but it can allow unwanted side effects you will have to check whereas value objects protect the code from such behaviors.

See you next time!


Image credits:

http://erikapov.blogspot.fr/2010/01/old-vs-new.html

The Law of Demeter

demeterDo not let the title mislead you, today’s article is not about greek mythology. As always I will speak about software development. In my last article I introduced the Tell Don’t Ask principle where it was about object behavior using encapsulation. The Law of Demeter (LoD) is an Object Oriented Programming (OOP) design guideline that fits well with this last principle. This practice uses encapsulation in order to reduce coupling between your components, and therefore it helps you improve your code quality. Here is a definition for the LoD:

Each unit should have only limited knowledge about other units: only units “closely” related to the current unit. Or: Each unit should only talk to its friends; Don’t talk to strangers.

For OOP it can be described with the followings list, which I consider easier to digest:

A method of an object may only call methods of:

  1. The object itself.
  2. An argument of the method.
  3. Any object created within the method.
  4. Any direct properties/fields of the object.

I will now demonstrate the use of the LoD with an example in C#. I will use the famous “The Paperboy and the Wallet” case from the original paper about the Demeter practice. In this example, the paperboy get a payment from a customer who as a wallet.

Initial situation

public class Wallet
{
    public float Value { get; set; }
 
    public void AddMoney(float amount)
    {
        Value += amount;
    }
 
    public void SubMoney(float amount)
    {
        Value -= amount;
    }
}
 
public class Customer
{
    public string FirstName { get; set; }
 
    public string LastName { get; set; }
 
    public Wallet Wallet { get; set; }
}
 
public class Paperboy
{
    public void SellPaper(Customer customer)
    {
        var payment = 2.0f;
        var wallet = customer.Wallet;
        if (wallet.Value >= payment)
        {
            wallet.SubMoney(payment);
        }
        else
        {
            // come back later
        }
    }
}

The Wallet class just stores its amount and exposes two methods to manipulate this amount. A customer has a first name, last name and a wallet. And finally the paperboy sell its good to a given customer using its wallet.

The problems

To me, it does not really looks like a real world scenario, I personally don’t give my wallet to every person I must pay. The paperboy knows that the customer has a wallet and he is able to manipulate it, this can be seen as a Single Responsibility Principle violation. And nothing prevents the paperboy code to do something like this:

customer.Wallet = null;

And maybe the customer’s wallet is already null, and therefore the paperboy’s method has to add a null check to avoid unwanted NullReferenceException.

public void SellPaper(Customer customer)
{
    var payment = 2.0f;
    var wallet = customer.Wallet;
    if (wallet != null)
    {
        if (wallet.Value >= payment)
        {
            wallet.SubMoney(payment);
        }
        else
        {
            // come back later
        }
    }
}

For a simple functionality, I think my code starts to look “heavier” than it should. A modification of the Wallet class will result in an update in the Paperboy class as well, the 3 classes are tightly coupled and it is unnecessary. The paperboy just wants to be paid, no matter if the money comes from a wallet or something else.

Following the Law of Demeter

I will now rewrite the code to fix the previous issues. To do so I will use encapsulation and add a PayAmount() method to the Customer class.

public class Wallet
{
    public Wallet(float initialAmount)
    {
        Value = initialAmount;
    }
 
    public float Value { get; private set; }
 
    public void AddMoney(float amount)
    {
        Value += amount;
    }
 
    public void SubMoney(float amount)
    {
        Value -= amount;
    }
}
 
public class Customer
{
    public Customer()
    {
        FirstName = "John";
        LastName = "Doe";
        _wallet = new Wallet(20.0f); // amount set to 20 for example
    }
 
    public string FirstName { get; private set; }
 
    public string LastName { get; private set; }
 
    private Wallet _wallet;
 
    public float PayAmount(float amountToPay)
    {
        if (_wallet.Value >= amountToPay)
        {
            _wallet.SubMoney(amountToPay);
            return amountToPay;
        }
        return 0;
    }
}
 
public class Paperboy
{
    public void SellPaper(Customer customer)
    {
        var payment = 2.0f;
        var amountPaid = customer.PayAmount(payment);
        if (amountPaid != payment)
        {
            // come back later
        }
    }
}

Now, the paperboy can only access to the first name and last name of the customer, without being able to modify them (for invoicing purposes for example). And he can tell the later to pay a certain amount for the paper without knowing that the money comes from a wallet, the LoD is also known as the principle of least knowledge. The relationship between the Paperboy class and the Wallet class has been removed, the code is less coupled.

With my refactoring, I increased the readability and the maintainability of my code without losing any functionality.

More than simple dot counting

Sometimes the Law of Demeter is stated with a simple phrase: “use only one dot“. This aims to focus on avoiding lines like the following, from the initial example:

customer.Wallet.Value;
customer.Wallet.SubMoney();

The risk with doing this is that you can have a NullReferenceException if the Wallet instance is not initialized, in these examples I used 2 dots. And here it is an indication that the LoD is violated.

But this is not always the case and this is why I consider that the rule should not be sum up to the simple “only one dot” principle. When working with the .NET framework and especially with Linq, it is easier to chain the calls and it has been designed to be used that way. I don’t consider that the following code sample breaks the LoD even if I use more than one dot.

enumerable.Where(o => o.LastName == "Doe").Select(o => o.FirstName).ToList();

The LoD is about loose coupling and encapsulation, it is not about dot counting.

This is the end of my presentation of the Law of Demeter, which I use a lot to avoid unnecessary coupling in my code. Remember that when working with an OOP language you are able to control the way you design your components to expose only what is required and nothing more.

See you next time!


Image credits:

http://sterendenn.deviantart.com/art/Art-Nouveau-Demeter-255076200

Tell Don’t Ask Principle

reported-ordersWhen working with an Object Oriented Programming (OOP) language, we design our classes based on their responsibility, following the Single Responsibility Principle (SRP) of course. We are able to use encapsulation to add behavior within these classes, unlike Procedural Programming (PP). The readability of the application components can be improved this way. Alec Sharp wrote about this topic:

Procedural code gets information then makes decisions. Object-oriented code tells objects to do things.

Am I saying that OOP is better than PP? Absolutely not, these are two different paradigms and it is important to remember this. I personally started with the procedural aspects, in C and I moved to OOP with C++ and after that I switch to C#. When I started C++ and the Object Oriented programming it was not as easy as it is now, it was a whole new “world”.

This is where the Tell Don’t Ask principle becomes handy. This principle helps you remember that you should design your components by focusing on their behavior and by hiding their internal working using encapsulation technic.

I have created an example of a component as an example to this principle. We will first take a look at the “ask” version and then we will see the “tell” version. This example is about a payment system that debit a wallet for a giving amount.

Ask version

public class Wallet
{
    public int OwnerId { get; set; }
 
    public int Balance { get; set; }
}
 
public class PaymentService
{
    public void DebitCustomer(int amount, int customerId)
    {
        var wallet = WalletRepository.GetWalletByCustomerId(customerId);
        if (wallet.Balance < amount)
            throw new Exception("Not enough funds.");
 
        wallet.Balance -= amount;
    }
 
    public void CreditCustomer(int amount, int customerId)
    {
        var wallet = WalletRepository.GetWalletByCustomerId(customerId);
        wallet.Balance += amount;
    }
}
 
public static class WalletRepository
{
    public static Wallet GetWalletByCustomerId(int customerId)
    {
        // Simulation of a query to data storage
        return new Wallet
        {
            Balance = 200,
            OwnerId = customerId
        };
    }
}

In this version my “Wallet” class is only a “data holder” and does not have a single piece of logic. It is the PaymentService that do all the work and “ask” the wallet if it has enough money to continue the operation. And it is the same service that update the wallet balance, it is not necessary the kind of behavior we might want in our application.

Now imagine that some customers are allowed to have a negative balance. In this case I do not want to throw an exception and I need to add a new condition in the DebitCustomer method.

public class Wallet
{
    public int OwnerId { get; set; }
 
    public int Balance { get; set; }
 
    public bool IsOverdraftAllowed { get; set; }
}
 
public class PaymentService
{
    public void DebitCustomer(int amount, int customerId)
    {
        var wallet = WalletRepository.GetWalletByCustomerId(customerId);
        if (wallet.Balance < amount && !wallet.IsOverdraftAllowed)
            throw new Exception("Not enough funds.");
        wallet.Balance -= amount;
    }
 
    public void CreditCustomer(int amount, int customerId)
    {
        var wallet = WalletRepository.GetWalletByCustomerId(customerId);
        wallet.Balance += amount;
    }
}

I had to modify both the Wallet class and the PaymentService class. I end up with a lot of wallet related logic in my payment service where it should only focus on debiting and crediting customers.

Now, it is time to see the tell version.

Tell version

public class Wallet
{
    public int OwnerId { get; private set; }
 
    public int Balance { get; private set; }
 
    public bool IsOverdraftAllowed { get; private set; }
 
    public Wallet(int ownerId, int balance, bool allowOverdraft)
    {
        OwnerId = ownerId;
        Balance = balance;
        IsOverdraftAllowed = allowOverdraft;
    }
 
    public void Debit(int amount)
    {
        if (Balance < amount && !IsOverdraftAllowed)
            throw new Exception("Not enough funds.");
        Balance -= amount;
    }
 
    public void Credit(int amount)
    {
        Balance += amount;
    }
}
 
public class PaymentService
{
    public void DebitCustomer(int amount, int customerId)
    {
        var wallet = WalletRepository.GetWalletByCustomerId(customerId);
        wallet.Debit(amount);
    }
 
    public void CreditCustomer(int amount, int customerId)
    {
        var wallet = WalletRepository.GetWalletByCustomerId(customerId);
        wallet.Credit(amount);
    }
}
 
public static class WalletRepository
{
    public static Wallet GetWalletByCustomerId(int customerId)
    {
        // Simulation of a query to data storage
        return new Wallet(customerId, 200, true);
    }
}

In this version, we can see that the payment service is much “cleaner” and it only focuses on its responsibility, nothing more. All the wallet logic has been moved to the Wallet class, where it belongs. And I used encapsulation to “protect” this class against unintentional uses, only the wallet instance can update its balance amount.

Tell Don’t Ask to save bandwidth

Now imagine that you have a class that act as a client for a service (like in WCF) and it calls a remote endpoint to perform a certain operation if it is available. I create the following piece of code as an example in the “ask” way.

public class RemoteService
{
    public bool IsOperationAvailable()
    {
        // some logic
        return true;
    }
 
    public void DoOperation()
    {
        // some operations
    }
}
 
public class Client
{
    public void CallRemote()
    {
        var service = new RemoteService();
        if (service.IsOperationAvailable()) // network latency here
            service.DoOperation();          // network latency again
    }
}

Even it the code does not look that “dirty”, I highlighted the issue in the comments. The client has to make two calls to the service in order to perform the desired operation. This has an effect on the application performance. In this case the “ask” approach clearly needs to be avoided.

The “Tell” approach

public class RemoteService
{
    private bool IsOperationAvailable()
    {
        // some logic
        return true;
    }
 
    public void DoOperation()
    {
        if (!IsOperationAvailable())
            return;
        // some operations
    }
}
 
public class Client
{
    public void CallRemote()
    {
        var service = new RemoteService();
        service.DoOperation();      // network latency only here
    }
}

I just made a slighty change to the code, now the client always tell the service to perform the operation and the service checks itself if the operation is available or not. This way I was able to reduce the number of calls on the network for my application without removing any functionalities.

You might wonder why I put an example like this one, that looks obvious. Simply because I have seen a similar example in a real world application. This way I wanted to show you the importance of the Tell Don’t Ask principle in some cases.

The Tell Don’t Ask principle helps you focus on the behavior of your classes and the functionalities you want them to expose. Remember that you don’t have to ask your components about their state in order to do an operation, just tell them to do it.

I hope you liked this presentation of this principle, as always, do not hesitate to share/comment/give your opinion.

See you next time!


Image credits:

https://www.englishclub.com/grammar/reported-orders.htm

SOLID: Dependency Inversion Principle

inversion-arrowsIt is time to see the fifth and last principle of SOLID: the Dependency Inversion Principle, also known as DIP. If you missed the other principles, you can learn more about them by following these links:

When developing software you will have a lot of different modules having each their role and responsibility. You will have to connect these modules between them in order to create the desired functionalities for your system. They will have dependencies between them and it will increase the coupling between your components. So it is important to reduce the risk involved in these dependencies. This is where the DIP rules come in play:

  1. High-level modules should not depend on low-level modules. Both should depend on abstractions.
  2. Abstractions should not depend upon details. Details should depend upon abstractions.

When a high-level module (business service for instance) depends on low-level module (data validator, a repository, …) it is difficult to reuse this module because it is highly coupled.

I have created an example that violates the DIP and I will show a way to solve the issue.

public class User
{
    public string UserName { get; set; }
    // some more properties
}
 
public class UserRepository
{
    public User GetUser(string userName)
    {
        // retrieve user in Database
        return new User();
    }
}
 
public class NotificationService
{
    public void Send(string message, User user)
    {
        // populate a message with the user information
        // send the message
    }
}
 
public class MessageSender
{
    // Send a message to a specific user
    public void SendMessage(string message, string userName)
    {
        var userRepository = new UserRepository();
        var user = userRepository.GetUser(userName);
 
        var notificationService = new NotificationService();
        notificationService.Send(message, user);
    }
}

In this piece of code my high-level module is the MessageSender, it allows me to send a given message to a given user (found by its username). For example this is a class that can be used from a user interface with a form. You can see that it has dependencies on a repository and a service, a modification of one of them can have impacts on my class. And what if I want to retrieve my user through a web service instead of the database? Or if I want to be able to send message to my friend via Twitter, Facebook or another social network? I will have to add complexity in this module even if its responsibility does not change from a “business” point of view.

My code is highly coupled and violates the Dependency Inversion Principle, I have to change it and I will by using abstraction (interfaces in my case).

public interface IUserRepository
{
    User GetUser(string userName);
}
 
public class UserRepository : IUserRepository
{
    public User GetUser(string userName)
    {
        // retrieve user in Database
        return new User();
    }
}
 
public interface INotificationService
{
    void Send(string mesage, User user);
}
 
public class NotificationService : INotificationService
{
    public void Send(string message, User user)
    {
        // populate a message with the user information
        // send the message
    }
}
 
public class MessageSender
{
    public INotificationService NotificationService { get; set; }
    public IUserRepository UserRepository { get; set; }
 
    // Send a message to a specific user
    public void SendMessage(string message, string userName)
    {
        var user = UserRepository.GetUser(userName);
        NotificationService.Send(message, user);
    }
}

In this new version I only use contracts/abstractions for my high-level component and this way I reduced the coupling with the implementations of the low-level modules. I decided to expose the dependencies via properties to access them and set them in order to clean the “SendMessage” method. When using this class I can now specify if I want to use an EmailNotificationService, a TwitterNotificationService, another social network related service or even a test double if I am in a test context.

Imagine creating automated tests with the first implementation! I would have to create an actual database for my UserRepository and a STMP service for my email based NotificationService. All of this only to test this little piece of logic, with abstraction and dependency inversion it is much easier now to test my components. A few months ago I introduced a DIP pattern known as Dependency Injection, you can learn more here.

This is the end of my presentation of the Dependency Inversion Principle and the end of the SOLID principles as well. I hope you like it and do not hesitate to leave a comment if you want to improve the concepts I have introduced in these blog posts.

See you next time!

SOLID: Interface Segregation Principle

multi-cablesIn my latest posts I introduced the Single Responsibility Principle, the Open Closed Principle and the Liskov Substitution Principle of SOLID. Now it is time to see the Interface Segregation Principle (ISP). In Object Oriented Programming (OOP) abstraction is a valuable asset especially with interfaces that allow you to design your application by contracts. Even if the interfaces do not contain any actual code it is important to control their size. The following sentence defines the ISP rule:

Clients should not be forced to depend on methods they do not use.

In other words it means that you should have small dedicated interfaces instead of larger ones. This way the client code will only have access to the functionalities it needs. I have created an example to show how the ISP can be violated and how to fix it.

public interface IRepository<T> where T : class
{
    void Insert(T entity);
    void Update(T entity);
    T Get(int id);
    void Delete(int id);
}

I have this interface that represents a repository to manipulate the entities stored in my database, it has the basic CRUD (Create Read Update Delete) operations. I will use this interface on my User entity:

public class User
{
    public int Id { get; set; }
    // other properties
}
 
public class UserRepository : IRepository<User>
{
    public void Insert(User entity)
    {
        // save user in DB
    }
 
    public void Update(User entity)
    {
        // update user in DB
    }
 
    public User Get(int id)
    {
        // retrieve user from DB
        return new User();
    }
 
    public void Delete(int id)
    {
        // delete user from DB
    }
}

Good, I have now a full control over my User objects lifetime in the database. Now I also want to be able to retrieve some event logs to be able to consult them, I will use my repository interface to do so.

public class EventLog
{
    public int Id { get; set; }
    public string Message { get; set; }
    // other properties
}
 
public class LogRepository : IRepository<EventLog>
{
    public void Insert(EventLog entity)
    {
        // nothing to do
    }
 
    public void Update(EventLog entity)
    {
        // nothing to do
    }
 
    public EventLog Get(int id)
    {
        // retrieve log from DB
        return new EventLog();
    }
 
    public void Delete(int id)
    {
        // nothing to do
    }
}

The issue is that my logs are stored in the database by other applications and in the one I’m working on I don’t have any logging to do. This force me to leave 3 methods empty since they are not used but are defined in my interface… This is clearly a violation of the Interface Segregation Principle. In this example I have a repository that is “read & write” and another that is just “read”. My abstraction is not correct in my context.

I have to find a solution that allows me to keep the functionalities for both repositories without breaking the ISP. I will then create two separate interfaces:

public interface IReadRepository<T> where T : class
{
    T Get(int id);
}
 
public interface IWriteRepository<T> where T : class
{
    void Insert(T entity);
    void Update(T entity);
    void Delete(int id);
}

As you can see one is dedicated to “read” operations and the other to “write” operations. My LogRepository can now inherits the IReadRepository because it does not need anything else.

public class LogRepository : IReadRepository<EventLog>
{
    public EventLog Get(int id)
    {
        // retrieve log from data source
        return new EventLog();
    }
}

And what about the UserRepository? Since it is “read & write” it will implement both interfaces. Multiple inheritance is commonly use when the ISP is in play.

public class UserRepository : IReadRepository<User>, IWriteRepository<User>
{
    public void Insert(User entity)
    {
        // save user in DB
    }
 
    public void Update(User entity)
    {
        // update user in DB
    }
 
    public User Get(int id)
    {
        // retrieve user from DB
        return new User();
    }
 
    public void Delete(int id)
    {
        // delete user from DB
    }
}

With time my application is likely to grow and use more and more entities from the database that will need “read” and “write” operations. If it is the case I can create an IReadWriteRepository that will be defined like this:

public interface IReadWriteRepository<T> : IReadRepository<T>, IWriteRepository<T>
    where T : class
{
 
}

In my example the UserRepository can implement this interface since it works as an alias over the two other ones. The ISP does not prevent you from regrouping interfaces under a common one. This will allow your code to be “cleaner” and explicit without losing functionalities.

This is the end of the Interface Segregation Principle presentation, remember to look for partial implementations of an interface if you want to spot where the ISP is violated.

I hope you liked this 4th principle of the SOLID series and as always do not hesitate to share, comment and give your opinion.

See you next time!

SOLID: Liskov Substitution Principle

lego-duploIt is time for the third entry for the SOLID series, after the SRP and the OCP, I’ll introduce the Liskov Substitution Principle (LSP). This concept has been introduced by Barbara Liskov in 1984, with Jeannette Wing they define this principle as the following:

Let q(x) be a property provable about objects x of type T. Then q(y) should be provable for objects y of type S, where S is a subtype of T.

Well, if you are like me you must be still trying to figure this one out to understand how it can be applied to Object Oriented Programming. Do not worry, a more accessible rule has been defined to specify the LSP.

Subtypes must be substitutable for their base types.

In other words it means that consuming any implementation of a base class should not change the correctness of the system. To give more depth to this principle I will use the famous Rectangle-Square example to show how this rule can be violated.

public class Rectangle
{
    public int Width { get; set; }
    public int Length { get; set; }
    public virtual int CalculateArea()
    {
        return Width * Length;
    }
}
 
public class Square : Rectangle
{
    public override int CalculateArea()
    {
        return Width * Width;
    }
}

As you can see, the Square is a subclass of Rectangle and that makes sense since a square is a rectangle with the same width and length. Now I will use these two classes in a program:

class Program
{
    static void Main(string[] args)
    {
        Rectangle rectangle = new Square();
        rectangle.Length = 10;
        rectangle.Width = 5;
        Console.WriteLine("Area : {0}", rectangle.CalculateArea());
    }
}
Area: 25

Let me resume the situation: I have a rectangle of 10 by 5 and its area is 25?! This does not make sense. Even if the inheritance seems legitimate, the way it is used in my application violates the Liskov Substitution Principle.

Using the NotSupportedException of the .NET framework is likely showing that the LSP is not respected in your source code. There is even a part of this same framework that transgress this principle.

class Program
{
    static void Main(string[] args)
    {
        ICollection<int> collection = new ReadOnlyCollection<int>(new List<int>{1,2});
        collection.Add(3);  // throws a NotSupportedException
    }
}

The ReadOnlyCollection does not allow any modification to the collection, it is impossible to add or remove items in it. Yet its the ICollection interface that defines methods to manipulate the items: the LSP is clearly violated.

The LSP is closely related to the Design by Contract approach to create software. This way your thinking ahead of the pre-conditions,  post-conditions and side effects for your application and every implementations of your subtypes are related to your contracts.

This is the end of the introduction to the Liskov Substitution Principle and especially how to detect when the rule is broken. I am still working on a relevant example to demonstrate how to start from a “bad” code sample, like the ones I gave you, to a sample that respect the LSP.

In the meantime do not hesitate to give your opinion regarding this topic.

See you next time!