HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Quiz Engine implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
enginequizimplementation

Problem

I'm trying to find a nice API for a Quiz Engine. This is what I have so far:

Implementation:

public class Question
{
    public string Title { get;set; }
    public List Answers { get; set; }

    public Answer CorrectAnswer { get; set; }
}

public class Answer
{
    public bool IsCorrect { get; set; }
    public string Content { get; set; }
}

public class QuizzEngine{
    Queue questions;

    public QuizzState QuizzState { get; set; }

    public QuizzEngine(IEnumerable questions)
    {
        this.questions = new Queue(questions);
        QuestionCount = this.questions.Count;
    }

    void StartQuizz()
    {
        QuizzState = QuizzState.Going;

        while(QuizzState == QuizzState.Going)
        {
            MoveToNextQuestion();
            if(QuizzState == QuizzState.Going) return;
            Answer answer = GetAnswer();
            TryAcceptAnswer(answer);
        }
    }

    Answer GetAnswer()
    {
        Answer answer;
        GetAnswerFromUI(out answer);
        return answer;
    }

    void GetAnswerFromUI(out Answer answer)
    {
        // somehow gets Answer through the UI
        answer = null;
    }

    public void TryAcceptAnswer(Answer answer)
    {
        var isCorrectAnswer = CurrentQuestion.CorrectAnswer.Equals(answer);
        if(isCorrectAnswer) CorrectAnswerCount++;
    }

    public void MoveToNextQuestion()
    {
        if(questions.Count == 0)
        {
            QuizzState = QuizzState.Finished;
            CurrentQuestion = null;
            return;
        }

        CurrentQuestion = questions.Dequeue();
    }

    public Question CurrentQuestion { get; private set; }
    public int CorrectAnswerCount { get; private set; }
    public int QuestionCount { get; private set; }
}

public enum QuizzState{
    Going,
    Finished
}


How could this be improved?

Solution

Good

  • Meaningful names are used for classes, methods, parameter and properties



  • using interfaces over implementation



  • using enums to signal a state



Not so good

  • Code isn't tested



  • Scope of property setters is public where they should be private



  • Using of properties which are never used



  • Single responsibility principle is violated



The good things are said, so let us focus on the bad things.

Code isn't tested

How / where should one see this ?

void StartQuizz()
{
    QuizzState = QuizzState.Going;

    while(QuizzState == QuizzState.Going)
    {
        MoveToNextQuestion();
        if(QuizzState == QuizzState.Going) return;
        Answer answer = GetAnswer();
        TryAcceptAnswer(answer);
    }
}


Here, after calling MoveToNextQuestion(), the code is checking the QuizzState property. So we assume the property has changed and if we take a look at this said method we see,

public void MoveToNextQuestion()
{
    if(questions.Count == 0)
    {
        QuizzState = QuizzState.Finished;
        CurrentQuestion = null;
        return;
    }

    CurrentQuestion = questions.Dequeue();
}


that the property is changed to QuizzState.Finished, if the question queue is empty. But the check is about if(QuizzState == QuizzState.Going) return;. So after the first call to MoveToNextQuestion() the code exists the while loop.

Scope of property setters and never used properties

As one of many samples let us take a look at the Answer class, so we can kill 2 birds with one stone

public class Answer
{
    public bool IsCorrect { get; set; }
    public string Content { get; set; }
}


The IsCorrect property is nowhere used in the whole code. But if it would be used somewhere, why should we allow to set / change the value of this property from a public place ? Especially for a Q&A engine, there shouldn't be a possibility to cheat !

Single responsibility principle

Where is the SRP viaolated ? Here

void GetAnswerFromUI(out Answer answer)
{
    // somehow gets Answer through the UI
    answer = null;
}


The QuizEngine shouldn't be responsible for any UI related stuff. But wait, how are we supposed to receive the answers and also how should we notify another object ( like an UI ) about the next question ?

That is what events and methods or properties are for.

To signal a changed state, like there is a question to be answered, an event should be used by the QuizEngine.

To receive an answer for a question, the QuizEngine should provide either a property setter or a method.

For decoupling the QuizEngine and the UI the QuizEngine should implement an interface which provides the event and the answer setting method/property.

Code Snippets

void StartQuizz()
{
    QuizzState = QuizzState.Going;

    while(QuizzState == QuizzState.Going)
    {
        MoveToNextQuestion();
        if(QuizzState == QuizzState.Going) return;
        Answer answer = GetAnswer();
        TryAcceptAnswer(answer);
    }
}
public void MoveToNextQuestion()
{
    if(questions.Count == 0)
    {
        QuizzState = QuizzState.Finished;
        CurrentQuestion = null;
        return;
    }

    CurrentQuestion = questions.Dequeue();
}
public class Answer
{
    public bool IsCorrect { get; set; }
    public string Content { get; set; }
}
void GetAnswerFromUI(out Answer answer)
{
    // somehow gets Answer through the UI
    answer = null;
}

Context

StackExchange Code Review Q#66587, answer score: 8

Revisions (0)

No revisions yet.