2
\$\begingroup\$

I have a method that generates some HTML, but it looks ugly to me. Maybe there are some better solutions for this?

 public static string GetReview(Quiz quiz)
    {
        StringBuilder sb = new StringBuilder();

        string questionSection = string.Format(
            "<div class=\"alert alert-info\" >" +
                "<strong style='color:#000000'>{0}</strong>" +
            "</div>", quiz.Question.GetText());


        string correctAnswer = quiz.Answer.GetText();
        string userAnswer = quiz.UserAnswer?.GetText() ?? string.Empty;

        string answerSection;
        if (correctAnswer == userAnswer) {
            answerSection = string.Format(
                "<p style='margin: 10px 0px 5px 400px;'>" +
                    "Your correct answer:" +
                 "</p>" +
                 "<div class='alert alert-success review-correct'>" +
                    "<strong>{0}</strong>" +
                 "</div>", correctAnswer);
        } else {
            answerSection = string.Format(
                @"<p style='margin: 10px 0px 5px 400px;'>" +
                    "The correct answer was:" +
                 "</p>" +
                 "<div class='alert alert-success review-correct'>" +
                    "<strong>{0}</strong>" +
                 "</div>" +
                 "<br/>" +
                 "You had selected: " +
                 "<span style='color:#a94442'>{1}</span>", correctAnswer, userAnswer);
        }

        string explanationSection = ForQuiz(quiz);

        sb.AppendFormat("<p>{0}</p><p>{1}</p><p>{2}</p>", questionSection, answerSection, explanationSection);

        return sb.ToString();
    }
\$\endgroup\$
2

1 Answer 1

5
\$\begingroup\$

Before we move to the generation part, let's talk about your html...

<p>
    <!-- questionSection -->
    <div class="alert alert-info" >
        <strong style='color:#000000'>question</strong>
    </div>
</p>
<p>
    <!-- answerSection: correct -->
    <p style='margin: 10px 0px 5px 400px;'>The correct answer was:</p>
    <div class='alert alert-success review-correct'>
        <strong>good answer</strong>
    </div>
    
    <br/>
    You had selected: <span style='color:#a94442'>bad answer</span>

    <!-- answerSection: wrong -->
    <p style='margin: 10px 0px 5px 400px;'>Your correct answer:</p>
    <div class='alert alert-success review-correct'><strong>qwe</strong></div>
</p>
<p>
    <!-- reviewSection -->
    <!-- ForQuiz(quiz) -->
</p>

  1. <p> can only nest phrasing content. <div> and <p> both belongs to flow content and palpable content. Therefore, you can't nested these 2 under <p> element. For the definition of a phrasing content consult this answer.
  2. Personally, I would prefer using <p> to separate block of text, instead of using a <br /> to insert a line-break.

    <!-- Question Section -->
    <div class="alert alert-info" >
        <strong style='color:#000000'>question</strong>
    </div>

    <!-- Answer Section: correct -->
    <p style='margin: 10px 0px 5px 400px;'>The correct answer was:</p>
    <div class='alert alert-success review-correct'>
        <strong>good answer</strong>
    </div>
    
    <p>You had selected: <span style='color:#a94442'>bad answer</span></p>

    <!-- Answer Section: wrong -->
    <p style='margin: 10px 0px 5px 400px;'>Your correct answer:</p>
    <div class='alert alert-success review-correct'>
        <strong>the answer</strong>
    </div>

    <!-- Review Section -->
    <!-- ForQuiz(quiz) -->

As for html generation, I believe, it is better to use a templating engine, such as RazorEngine which allows you to write C# code within the template itself :

review.cshtml :

@{
    var correctAnswer = Model.Answer.GetText();
    var userAnswer = Model.UserAnswer == null ? Model.UserAnswer.GetText() : string.Empty;
}

<!-- Question Section -->
<div class="alert alert-info">
    <strong style='color:#000000'>@Model.Question.GetText()</strong>
</div>

<!-- Answer Section -->
@if (userAnswer == correctAnswer)
{
    <p style='margin: 10px 0px 5px 400px;'>The correct answer was:</p>
    <div class='alert alert-success review-correct'>
        <strong>@correctAnswer</strong>
    </div>
    <p>You had selected: <span style='color:#a94442'>@userAnswer</span></p>
}
else
{
    <p style='margin: 10px 0px 5px 400px;'>Your correct answer:</p>
    <div class='alert alert-success review-correct'>
        <strong>@correctAnswer</strong>
    </div>
}

<!-- Review Section -->
<!-- TODO: implement ForQuiz(quiz) in razor syntax... -->

generation code :

public string GetReview(Quiz quiz)
{
    var path = Desktop.GetFile("review.cshtml"); // TODO: fix this path
    var template = File.ReadAllText(path);

    return Engine.Razor.RunCompile(template, path, typeof(Quiz), quiz);
}

EDIT: I noticed you used the null-propagation operator from c# 6. You can also use the new string interpolation to format the html :

public static string GetReview3(Quiz quiz)
{
    var builder = new StringBuilder();
    builder.Append($@"
        <div class='alert alert-info'>
            <strong style='color:#000000'>{quiz.Question.GetText()}</strong>
        </div>
    ");

    string correctAnswer = quiz.Answer.GetText();
    string userAnswer = quiz.UserAnswer?.GetText() ?? string.Empty;
    if (correctAnswer == userAnswer)
    {
        builder.Append($@"
            <p style='margin: 10px 0px 5px 400px;'>The correct answer was:</p>
            <div class='alert alert-success review-correct'>
                <strong>{correctAnswer}</strong>
            </div>
            <p>You had selected: <span style='color:#a94442'>{userAnswer}</span></p>
        ");
    }
    else
    {
        builder.Append($@"
            <p style='margin: 10px 0px 5px 400px;'>Your correct answer:</p>
            <div class='alert alert-success review-correct'>
                <strong>{correctAnswer}</strong>
            </div>
        ");
    }

    builder.Append(ForQuiz(quiz));


    return builder.ToString();
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.