.NET class design question

Question

I have a class called Question that has a property called Type. Based on this type, I want to render the question to html in a specific way (multiple choice = radio buttons, multiple answer = checkboxes, etc...). I started out with a single RenderHtml method that called sub-methods depending on the question type, but I'm thinking separating out the rendering logic into individual classes that implement an interface might be better. However, as this class is persisted to the database using NHibernate and the interface implementation is dependent on a property, I'm not sure how best to layout the class.

The class in question:

public class Question
{
    public Guid ID { get; set; }
    public int Number { get; set; }
    public QuestionType Type { get; set; }
    public string Content { get; set; }
    public Section Section { get; set; }
    public IList<Answer> Answers { get; set; }
}

Based on the QuestionType enum property, I'd like to render the following (just an example):

<div>[Content]</div>
<div>
   <input type="[Depends on QuestionType property]" /> [Answer Value]
   <input type="[Depends on QuestionType property]" /> [Answer Value]
   <input type="[Depends on QuestionType property]" /> [Answer Value]
   ...
</div>

Currently, I have one big switch statement in a function called RenderHtml() that does the dirty work, but I'd like to move it to something cleaner. I'm just not sure how.

Any thoughts?

EDIT: Thanks to everyone for the answers!

I ended up going with the strategy pattern using the following interface:

public interface IQuestionRenderer
{
    string RenderHtml(Question question);
}

And the following implementation:

public class MultipleChoiceQuestionRenderer : IQuestionRenderer
{
    #region IQuestionRenderer Members

    public string RenderHtml(Question question)
    {
        var wrapper = new HtmlGenericControl("div");
        wrapper.ID = question.ID.ToString();
        wrapper.Attributes.Add("class", "question-wrapper");

        var content = new HtmlGenericControl("div");
        content.Attributes.Add("class", "question-content");
        content.InnerHtml = question.Content;
        wrapper.Controls.Add(content);

        var answers = new HtmlGenericControl("div");
        answers.Attributes.Add("class", "question-answers");
        wrapper.Controls.Add(answers);

        foreach (var answer in question.Answers)
        {
            var answerLabel = new HtmlGenericControl("label");
            answerLabel.Attributes.Add("for", answer.ID.ToString());
            answers.Controls.Add(answerLabel);

            var answerTag = new HtmlInputRadioButton();
            answerTag.ID = answer.ID.ToString();
            answerTag.Name = question.ID.ToString();
            answer.Value = answer.ID.ToString();
            answerLabel.Controls.Add(answerTag);

            var answerValue = new HtmlGenericControl();
            answerValue.InnerHtml = answer.Value + "<br/>";
            answerLabel.Controls.Add(answerValue);
        }

        var stringWriter = new StringWriter();
        var htmlWriter = new HtmlTextWriter(stringWriter);
        wrapper.RenderControl(htmlWriter);
        return stringWriter.ToString();
    }

    #endregion
}

The modified Question class uses an internal dictionary like so:

public class Question
{
    private Dictionary<QuestionType, IQuestionRenderer> _renderers = new Dictionary<QuestionType, IQuestionRenderer>
    {
        { QuestionType.MultipleChoice, new MultipleChoiceQuestionRenderer() }
    };

    public Guid ID { get; set; }
    public int Number { get; set; }
    public QuestionType Type { get; set; }
    public string Content { get; set; }
    public Section Section { get; set; }
    public IList<Answer> Answers { get; set; }

    public string RenderHtml()
    {
        var renderer = _renderers[Type];
        return renderer.RenderHtml(this);
    }
}

Looks pretty clean to me. :)

Answer 1

Generally speaking, whenever you see switches on a Type or Enum, it means you can substitute in objects as the "Type" - said differently, a case for polymorphism.

What this means practically is that you'll create a different class for each Question type and override the RenderHTML() function. Each Question object will be responsible for knowing what input type it ought to output.

The benefits are that you remove the switch statement as well as produce good OO based code. The draw backs are that you add a class for every Question type (in this case minimal impact.)

Answer 2

You can for example use the strategy pattern:

1. Have all your HTML renderers implement a common interface, for example IQuestionRenderer, with a method name Render(Question).

2. Have an instance of Dictionary<QuestionType, IQuestionRenderer> in your application. Populate it at initialization time, perhaps based on a configuration file.

3. For a given instance of a question, do: renderers[question.Type].Render(question)

Or, you could have methods named RenderXXX where XXX is the question type, and invoke them by using reflection.

Answer 3

This is a classic case for using object inheritance to achieve what you want. Anytime you see a big switch statement switching on the type of an object, you should consider some form of subclassing.

I see two approaches, depending on how "common" these question types really are and whether rendering is the only difference between them:

Option 1 - Subclass the Question class

public class Question
{
    public Guid ID { get; set; }
    public int Number { get; set; }
    public string Content { get; set; }
    public Section Section { get; set; }
    public IList<Answer> Answers { get; set; }

    public virtual string RenderHtml();
}

public class MultipleChoiceQuestion 
{
    public string RenderHtml() { 
      // render a radio button
    }
}

public class MultipleAnswerQuestion 
{
    public string RenderHtml() { 
      // render a radio button
    }
}

Option 2 - Create a render interface, and make that a property on your question class

public class Question
{
    public Guid ID { get; set; }
    public int Number { get; set; }
    public string Content { get; set; }
    public Section Section { get; set; }
    public IList<Answer> Answers { get; set; }

    public IRenderer Renderer { get; private set; }
}

public interface IRenderer {
    void RenderHtml(Question q);
}

public class MultipleChoiceRenderer : IRenderer
{
    public string RenderHtml(Question q) { 
      // render a radio button
    }
}

public class MultipleAnswerRenderer: IRenderer
{
    public string RenderHtml(Question q) { 
      // render checkboxes
    }
}

In this case, you would instantiate the renderer in your constructor based on the question type.

Option 1 is probably preferable if question types differ in more ways than rendering. If rendering is the only difference, consider Option 2.

Answer 4

It's a good idea to separate the rendering logic into its own class. You don't want rendering logic embedded into the business logic of your application.

I would create a class called QuestionRenderer that takes in a Question, reads its type, and outputs rendering accordingly. If you're using ASP.NET it could output webcontrols, or you could do a server control that outputs HTML.