Thursday, September 22, 2016

Improving software design using SOLID & GRASP principles



In this article I am going to improve the design of a class and show how some SOLID and GRASP principles help in this process.

Preview
I will have a class Fibb that does some computation (Fibonacci sequence)
Then I will add a new capability to this class (printing the result) in the simplest way.
We will notice that there are issues with this way of adding the capability, mainly because the class Fibb will be cluttered with a lot of code that will be difficult to maintain and extend.
To improve the code, I will delegate the task of printing to another class, Printer.
Then we will notice that Printer is bearing more information than needed.
I will use polymorphism to improve the use of Printer class.
The final design will be checked against some principles from SOLID and GRASP to show that it is a good design.


Before I start
Fibonacci sequence is a series of numbers.
Each number is found by adding the two numbers before it in the sequence.
The 1st and the 2nd Fibonacci numbers are given/known by default and they are, usually, 0 and 1.
The 3rd Fibonacci number is found by summing up the 1st and 2nd numbers, so its value is 1
The 4th Fibonacci number is found by summing up the 2nd and 3rd numbers, so its value is 2
And so on ...

A simple class
Let's consider the simplest version of Fibonacci sequence computing class (check http://www.rosettacode.org/wiki/Fibonacci_sequence for more robust versions)
The class offers a function that, given a number/rank, will find the corresponding Fibonacci number.
#include <iostream>
using namespace std;

class Fibb
{
    public:
    int operator()(int n)
    {
        if(n<0)
        {
            cout << "this case is not supported!"<<endl;
            return 0;
        }
   
        if(n==0)
            return 0;
       
        if(n==1)
            return 1;
       
        int fnow = 0, fnext = 1, tempf;

        while(--n>0)
        {
            tempf = fnow + fnext;
            fnow = fnext;
            fnext = tempf;
        }

        return fnext;   
    }

};

int main()
{
    int r;
    Fibb fibb;

    r = fibb(0);
    cout << r << endl;
    r = fibb(1);
    cout << r << endl;
    r = fibb(2);
    cout << r << endl;
    r = fibb(3);
    cout << r << endl;
}




Adding the printing capability
Now, we want the computing class to print the result, instead of the client doing it (the function main above). So we'll add a function OutputResult


The code will look like the highlighted below:


#include <iostream>
using namespace std;

class Fibb
{
    public:
    int operator()(int n)
    {
        if(n<0)
        {
            cout << "this case is not supported!"<<endl;
            return 0;
        }
   
        if(n==0)
            return 0;
       
        if(n==1)
            return 1;
       
        int fnow = 0, fnext = 1, tempf;

        while(--n>0)
        {
            tempf = fnow + fnext;
            fnow = fnext;
            fnext = tempf;
        }

        OutputResult();
         
        return fnext;    
    }
    void OutputResult()
    {
        cout << fnext << endl;
    }

};

int main()
{
    Fibb fibb;
    fibb(0);
    fibb(1);
    fibb(2); 
    fibb(3);

}

But this code is not perfect from the point of view of OOP: What if we want to offer the possibility to output the result to a file as well ?

Printing (version 2)

One solution is adding a variable to the class Fibb that lets it decide how to print the result:


class Fibb 
{

public:
    enum class DestinationFlag
    {
        e_toScreen,
        e_toFile,
    };
private:
    DestinationFlag     m_df;  //the variable that decides how to print the result
    fostream            m_os;
  
public:  
    bool Init(DestinationFlag df, const char* filename=nullptr)
    {
        bool res=true;
        m_df = df;
      
        if(df == e_toFile)
        {
            //check filename parameter
            //try to open file
            // manage file management errors
            // and set m_os and/or res
          
        }
      
        return res;
    }  
  
    void OutputResult()
    {
        if(m_df == e_toFile)
            m_os << fnext << endl;
        else
            cout << fnext << endl;
    }

    int operator()(int n)
    {

       ...
};

int main()

{
    FibbToFile();
    FibbToScreen();
}

void FibbToFile()

{
    Fibb fibb;

    if(!fibb.Init(e_toFile, "a_file_to_output_to.txt"))

        return;
      
    fibb(5);
}

void FibbToScreen()

{
    Fibb fibb;

    fibb.Init(e_toScreen);

    fibb(3);
}


Issues in version 2

Offering the possibility to print the result to a file, in this way, has many problems.
This is because Fibb class is now doing more than it should.
Not only it's doing its core task, which is to compute Fibonacci sequence, it is now also :
- maintaining the way to print the result, through the member m_df.
- managing the stream to output to m_os and all its related issues (file no found, file can't be created, ...). This will also lead to defining function members just for this task.
- the member m_os is always there in Fibb objects even when they are printing to the screen. This is a waste of memory space.


Another problem with this way is that Fibb starts to be difficult to test. A key technique in testing code is isolation. To have reliable tests of a functionality, it should be easy to isolate from other parts and functionalities of a system.
In our case, how would we test the correct handling of the output file management and check its different errors without modifying Fibb just for testing this functionality ?


Issues in version 2 from SOLID and GRASP's perspective

We can also detect issues with this solution by checking it against some SOLID and GRASP principles:

- The 'S' in SOLID principle states that a class should have a single responsibility.
In our case, Fibb is breaking this rule because it is also managing printing.

Fibb is also breaking the "high cohesion" in GRASP. The roles of the members in Fibb are not coherent; there is a member to compute Fibonacci sequence, another to decide of the way printing should happen and other members to manage a file.


- The 'O' in SOLID states that the capabilities of a class/system should be open to extension and closed to modification.

In our case, we can not extend the capability of printing to memory or extend it to send the result through network without modifying the class Fibb.


Let's improve the code.


Delegating printing


Let's do a high level analysis.

The key idea we should use to solve this kind of problems is to say:
1- Fibb calculates Fibonacci sequence.
2- Fibb signals an object/code to print something (the result n our case)
3- Fibb should ideally know nothing about the object it asks to print data
4- The object printing data should know nothing about Fibb. A direct implication of this is:
4.1-the object printing data should be able to print anything passed on to it from any class/code.
4.2-any change in Fibb should not affect the object, and vise-versa

All these points can be achieved by delegating the print operations to a different class:




In terms of code, it will look like:

class Fibb
{
 

    Printer  *  m_printer;  
public:  
    Fibb(Printer *p = nullptr):m_printer(p) { } 


    void OutputResult()

    {
        if(m_printer)
            m_printer->Print(fnext); 
    }
    int operator()(int n)
    {
     ...
};

The issue here is that Printer needs to know where to print data: screen or file.

To achieve this we can have Printer::print have a code like :


void Printer::Print()
{
    if(m_df == e_toFile)
        m_os << fnext << endl;
    else
        cout << fnext << endl;
}

and the class will have a member m_df that is set at construction to either e_toFile or e_toScreen.



The class Printer will be defined as:

class Printer
{
    DestinationFlag     m_df;  //the variable that decides how to print the result
    fostream                m_os;
  
public:  
    void Print();
    bool Init(DestinationFlag df, const char* filename=nullptr)
    {
        bool res=true;
        m_df = df;
      
        if(df == e_toFile)
        {
            //check filename parameter
            //try to open file
            // manage file management errors
            // and set m_os and/or res
          
        }
      
        return res;
    }  
  
};

But we have another problem here also.
When Printer class is instantiated to print to the screen, any code and field related to printing to file is not going to be used, but it will still be part of the instance.
And when it is instantiated to print to a file, any code and field related to printing to the screen is not going to be used.
This appears clearly in the Printer::Print where we branch following the state of the object.

Polymorphism


Fibb needs to use a reference to only one class that can represent either classes.

It means to have a base class having all the common code and/or field, and to derive from it classes specialised to do each a specific and a different task.
In our case, Fibb will reference Printer, which will be the base class for PrinterToScreen and PrinterToFile classes. These will specialise any code they need.





These classes will have the following implementation


class Printer
{
public:
    virtual ~Printer(){}
    virtual void Print(int n)=0;
};


class PrinterToScreen : public Printer

{
public:
    void Print(int n) override
    {
        cout << n << endl;
    }   
};

class PrinterToFile : public Printer

{
    ofstream f;
   
public:
    enum class eStatus
    {
        Failure,
        Success
    };
public:
    ~PrinterToFile()
     {
         f.close();
     }
    eStatus Init(const char * filename)
    {
            //check filename parameter
            //try to open file
            // manage file management errors
            //return eStatus::Failure or eStatus::Success
    }
    void Print(int n) override
    {
        f << n << endl;
    }   
};


Notice how all the code related to printing to file is now located in a separate class. So when an object of PrinterToFile in instantiated to print to a file, it does not have any superfluous data or code.
The same goes for objects of PrinterToScreen class. 


And this is how to use this new design






int main()
{
    FibbToFile();
    FibbToScreen();
}

void FibbToFile()
{
     PrinterToFile pf;

    if(pf.Init("a_file_to_output_to.txt") != PrinterToFile::eStatus::Success)

        return;

     Fibb fibb(&pf);

     
    fibb(4);
   
}

void FibbToScreen()

{
    PrinterToScreen ps;
    Fibb fibb(&pc);

    fibb(3);


}


Verification of the design 
from SOLID and GRASP's perspective

Let's check this design against some of the patterns/rules in SOLID and GRASP principles:


-Single Responsibility Principle: Each class in the design has one and only one responsibility.


-Dependency Inversion Principle: Instead of Fibb depending directly on the class printing to the screen and/or the one printing to a file, it only depends on an interface.

An abstraction layer has been introduced between Fibb and PrinterToScreen and PrinterToFile.

-Open Closed Principle: If we need to extend the capability of the system so we can print to memory, we can easily do it by deriving a new class from Printer and override Print function. The new design is open to extension while being closed to modification to any existing classes.



-High Cohesion: each class has only members that collaborate toward one goal


-Information expert: Each class has all the necessary members to fulfil its task. None of them needs anything from its peers.


-Low coupling: it is possible to change how the PrinterToScreen does its job without the need of altering Fibb and vise-versa. The same goes for the rest.


-Protected variations: same remark as for open closed principle above.

-Polymorphism: No manual branching is done to execute a code based on the setting of Printer, as was done before:

void Printer::Print()
{
    if(m_df == e_toFile)
        m_os << fnext << endl;
    else
        cout << fnext << endl;
}

This branching is replaced by the creation of the proper object, an instance of PrinterToScreen or of PrinterToFile.

Conclusion

We've gone from Fibb taking in charge a task (printing) not belonging to its core job, to delegating the task to another class (Printer), to making Printer a representative of concrete printing classes.

The final version of the design allows for easy extension and easy testing of each class in isolation.