tags:

views:

431

answers:

11

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. :)

+1  A: 

Why not have a QuestionRenderer class (actually, it will be a control) which exposes a Question as a property which you can set.

In the render method, you can decide what to render based on the question type.

Winston Smith
I was actually thinking something similar with interfaces, where each implementation of the interface would handle a specific QuestionType
Chris
+1  A: 

I don't like the idea of rendering details being in the same class as the data.

So, one option would be to have your rendering method simply generate one of a set of user controls that handled the actual HTML rendering.

Another would be to have a separate class QuestionRenderer which would have the various subclasses for question types (each of which would render the correct HTML).

JacobM
+3  A: 

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.

Dave Swersky
+12  A: 

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.)

Gavin Miller
+1 for polymorphism over switch statement. Also promotes Single Responsibility.
Dave Swersky
@Dave: You have to switch somewhere... with inheritance you just move it to instantiation. I'd prefer composition (strategy pattern like in Konamiman answer below)
Kai
Konamiman answer's ABOVE now, cause it's the right one ... says Chris.
Kai
@kai - the only reason to prefer inheritance here (or use it in conjunction with composition) is that the different Question widgets appear to be substitutable. Moreover, most of the html that needs to be rendered will be common across the different display types.
Jeff Sternal
A: 

Stating the obvious: You could probably use a factory method to get the instance of the required rendered class and call render on that to get the required output.

Krishna Kumar
+4  A: 

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.

Ryan Brunner
I thought about subclassing question itself, but I'm not sure how to make NHibernate return a specific sub-type based on the property type. Ideally I don't want to do a bunch of casting after I've retrieved the object from the database.
Chris
In that case the strategy pattern (option 2) would be ideal. You can keep the Question Type as an internal or private property (or public if there's still a need for it), and instantiate your IRenderer based on that value.
Ryan Brunner
+11  A: 

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.

Konamiman
#1 is exactly the path I have started down as I wait for answers. Sounds like I might be on the right track...
Chris
A: 

The approach I would take is to create a separate Control (or a HtmlHelper method if you're in MVC) for each visual style you would like your questions to be rendered using. This separates the concerns of representing the question as an object and representing it visually neatly.

Then you can use a master Control (or method) to choose the correct rendering method based on the type of the Question instance presented to it.

Programming Hero
+1  A: 

The rendering is definitely a UI concern, so I'd separate that from the Question class and add a factory to isolate the switching logic (the QuestionControl base class inherits from WebControl and would contain the majority of the rendering logic):

RadioButtonQuestionControl: QuestionControl {
    // Contains radio-button rendering logic
}

CheckboxListQuestionControl: QuestionControl {
    // Contains checkbox list rendering logic
}

QuestionControlFactory {
    public QuestionControl CreateQuestionControl(Question question) {
       // Switches on Question.Type to produce the correct control
    }
}

Usage:

public void Page_Load(object sender, EventArgs args) {
    List<Question> questions = this.repository.GetQuestions();
    foreach(Question question in Questions) {
        this.Controls.Add(QuestionControlFactory.CreateQuestionControl(question));
        // ... Additional wiring etc.
    }
}
Jeff Sternal
If you add this to the UserType the control is created for you.
AndrewB
I haven't explored NHibernate enough yet, but directly mapping to UI controls sounds extremely interesting.
Jeff Sternal
+1  A: 

I think what you want is an IUserType that convert the property from the hibernate mapping to the correct control type via some Question factory.

An example of the use of an IuserType can be found here: NHibernate IUserType

in the example it converts a blob to an image for use on the client side but with the same idea you can make your page created with the QuestionType.

AndrewB
A: 

You could use the strategy pattern (Wikipedia) and a factory in combination.

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; }

    private IQuestionRenderer renderer;

    public RenderHtml()
    {
         if (renderer == null)
         {
              QuestionRendererFactory.GetRenderer(Type);
         }
         renderer.Render(this);
    }
}


interface IQuestionRenderer
{
    public Render(Question question);
}


public QuestionRendererA : IQuestionRenderer
{
    public Render(Question question)
    {
         // Render code for question type A
    }
}

public QuestionRendererB : IQuestionRenderer
{
    public Render(Question question)
    {
         // Render code for question type B
    }
}

public QuestionRendererFactory
{
    public static IQuestionRenderer GetRenderer(QuestionType type)
    {
        // Create right renderer for question type
    }
}

Only the public properties need to be included in NHibernate.

spa
Damn, seems I write too slow :-)
spa