Search code examples
solid-principlessingle-responsibility-principleopen-closed-principle

Single responsability principle vs open close principle


I'm writting a program to show a collection of questions to an user, collect his responses and print them.

I have different types of questions depending on the kind of response they require: integer, boolean or text.

I started writing this code:

abstract class Question
{
    string text;
}

class IntegerQuestion : Question
{
    int response;
}

class TextQuestion : Question
{
    string response;
}

class BooleanQuestion : Question
{
    bool response;
}

Well, now we have to print the questions and responses.

My first approach was to define a new abstract Print function in Question class to force subclasses to define the Print method, and then a Printer class:

abstract class Question
{
    string text;
    abstract string Print();
}

class Printer
{
    string PrintQuestions(List<Question> questions)
    {
        string result = "";

        foreach(var question in Questions)
            result += question.Print() + "\r\n";

        return result;
    }
}

Another approach I thought about was to forgive the abstract method and create Printer class like this:

class Printer
{
    string PrintQuestions(List<Question> questions)
    {
        string result = "";

        foreach(var question in Questions)
        {
            if(question is IntegerQuestion)
            {
                var integerQuestion = (IntegerQuestion)question;
                result += integerQuestion.text + integerQuestion.response;
            }
            if(question is TextQuestion)
            {
                ...
            }
            ...
        }

        return result;
    }
}

Clearly, the second approach doesn't follow the OCP for Printer class and first does it.

But, what about SRP?

If then I need to write questions and responses in HTML:

abstract class Question
{
    string text;
    abstract string Print();
    abstract string PrintHTML();
}

class HTMLPrinter { ... }

¿Aren't question subclasses violating SRP because they know how to print them in plain text and html?


Solution

  • Aren't question subclasses violating SRP because they know how to print them in plain text and html

    You're perfectly right.

    First, on your naming convention and your design, if I understand your demo, why answers extends Question ? Inheritence is a "Is a" relation between object.

    Should we said that an Answer is a Question ? Looks like there are two different concepts in your business:

    • Question wich hold the question
    • Answer wich hold the user answer to the Question

    I'll probably do something like : (sorry for the syntax, it's some sort of pseudo-code)

    interface IAnswer{
        string toString();
    }
    class IntegerAnswer implements IAnswer{
        int answer;
        string toString(){
            return (string)this.answer;
        }
    }
    ....
    class Question{
        string text;
        IAnswer answer; //or List<IAnswer> answers if you can old more than one answer by Question
        string toString(){
            return this.text;
        }
    }
    

    Then, you can define a printer :

    interface IQuestionPrinter{
        string print(List<Question> questions);
    }
    class Printer implements IQuestionPrinter{
         string print(List<Question> questions){
              string res = '';
              foreach(question in questions){
                  res+=question.toString() + " : " + question.answer.toString();
              }
              return res;
         }
    }
    class HTMLPrinter implements IQuestionPrinter{
        string print(List<Question> questions){
              string res = "<ul>";
              foreach(question in questions){
                  res+="<li>";
                  res+= "<span>" + question.toString() + "</span>";
                  res+="<span>" + question.answer.toString()+"</span>;
                  res+="</li>";
              }
              return res+"</ul>";
         }
    }
    

    Or something like that.

    Then all your Questions and Answers knows about that they have to extends a toString() method, and we delegate the print work to a dedicated IQuestionPrinter.

    Making the Answer interface is good since Printer didn't have to know if an Answer is Integer, Boolean or String or Whatever. And if you have other "types" of Question, you should define an interface IQuestion :

    interface IQuestion{
        IAnswer answer; // or List<IAnswer> answers
        string toString();
    }
    

    And then the IQuestionPrinter should take it in consideration :

    interface IQuestionPrinter{
        string print(List<IQuestion> questions);
    }