Multiple-choice questionnaire

Question

I'm new to C# and for my assignment I have to design a multiple choice questionnaire where the student can login and take the test, view the memo, etc. I'm not very concerned about the GUI, but rather the structure of my classes and code. I will be using the StudentController class to interact with the GUI. It's a rough but here's my code so far:

Student class

public class Student : User
{
    private int finalMark;
    private List<int> answers;

    public Student()
    {
        answers = new List<int>();
    }

    /// <summary>
    /// Gets the final mark for the student
    /// </summary>
    public int GetFinalMark() => finalMark;

    /// <summary>
    /// Sets the final mark for the student
    /// </summary>
    public void SetFinalMark(int value) => finalMark = value;

    /// <summary>
    /// Fetches the answer the student chose for the indexed question
    /// </summary>
    /// <param name="index">The index of the question to retrieve the student's answer from</param>
    /// <returns>The index of the answer the user chose</returns>
    public int GetAnswer(int index) => answers[index];

    /// <summary>
    /// Sets the answer at the index's position
    /// </summary>
    /// <param name="index">The index to set the answer at</param>
    /// <param name="answer">The answer to set</param>
    public void SetAnswer(int index, int answer) => answers[index] = answer;

    /// <summary>
    /// Adds an answer to the student's list of answered questions
    /// </summary>
    /// <param name="answer">The answer to add to the list of answered questions</param>
    public void AddAnswer(int answer) => answers.Add(answer);
}

Multiple choice question

public class MultipleChoiceQuestion
{
    //  The question that is stated in the multiple choice question
    private string question;

    //  The list of possible answers to the question
    private string[] choices;

    //  The index of the correct answer, ie. choices[answer]
    private int answer;

    public MultipleChoiceQuestion()
    {
        choices = new string[4];
    }

    /// <summary>
    /// Gets and sets the question
    /// </summary>
    public string Question { get => question; set => question = value; }

    /// <summary>
    /// Gets the indexed question as a string
    /// </summary>
    /// <param name="index"></param>
    /// <returns></returns>
    public string GetChoices(int index) => choices[index];

    /// <summary>
    /// Sets the choice at the current index, eg. choice[0] = "Static void"
    /// </summary>
    /// <param name="index">The index of the choice</param>
    /// <param name="value">The choice to set</param>
    public void SetChoice(int index, string value) => choices[index] = value;

    /// <summary>
    /// Gets and sets the correct answer as an index
    /// </summary>
    public int Answer { get => answer; set => answer = value; }
}

Test Class

    public class Test
{
    private List<MultipleChoiceQuestion> question;

    public Test()
    {
        question = new List<MultipleChoiceQuestion>(); ;
    }

    /// <summary>
    /// Returns the question text at the given index of questions
    /// </summary>
    /// <param name="index">The index of the question to retrieve</param>
    /// <returns>A string of the question at the current index</returns>
    public string GetQuestionText(int index) => question[index].Question;


    public void SetQuestion(int index, string value) => question[index].SetChoice(index, value);

    /// <summary>
    /// Adds a new question to the list
    /// </summary>
    /// <param name="newQuestion">The question to add to the list</param>
    public void AddQuestion(MultipleChoiceQuestion newQuestion) => question.Add(newQuestion);

    /// <summary>
    /// Retries one of the choices for the question. Eg. choice A, choice B, choice C...
    /// </summary>
    /// <param name="choice">The index of the choice, eg. choice 2</param>
    /// <param name="index">The index of the question</param>
    /// <returns></returns>
    public string GetChoice(int choice, int index) => question[index].GetChoices(choice);

    /// <summary>
    /// Retrieves the index of the answer to the question
    /// </summary>
    /// <param name="questionIndex">The index of the answer</param>
    /// <returns></returns>
    public int GetAnswerIndex(int questionIndex) => question[questionIndex].Answer;

    /// <summary>
    /// Gets how many questions there are in the test
    /// </summary>
    /// <returns>The amount of questions in the test</returns>
    public int GetSize()
    {
        return question.Count;
    }
}

Student Controller

    public class StudentController
{
    private List<Student> students;
    private FileManager fileManager;
    private Test test;

    public StudentController()
    {
        students = new List<Student>();
        fileManager = new FileManager();
        test = new Test();
        Init();
    }

    private void Init()
    {
        students = fileManager.LoadStudents();
        test = fileManager.LoadTest();
    }

    /// <summary>
    /// Verifies that the login details match what is in the system
    /// </summary>
    /// <param name="username">The username of the student</param>
    /// <param name="password">The password of the student</param>
    /// <returns>True if the credentials match that on the system</returns>
    public bool CheckLogin(int username, string password)
    {
        bool credentialsMatch = false;
        foreach (Student student in students)
        {
            if (student.ID == username && student.Password == password)
            {
                credentialsMatch = true;
                break;
            }
        }
        return credentialsMatch;
    }

    /// <summary>
    /// Fetches the index of the student in a list
    /// </summary>
    /// <param name="ID">The ID of the student</param>
    /// <returns>The student's ID if found, else returns -1</returns>
    public int GetStudentIndex(int ID)
    {
        int id = -1;
        foreach (Student student in students)
        {
            if (student.ID == ID)
            {
                id = ID;
                break;
            }
        }
        return id;
    }

    /// <summary>
    /// Sets the final mark of the student
    /// </summary>
    /// <param name="studentIndex">The index of the student</param>
    /// <param name="mark">The final mark of the student</param>
    public void SetFinalMark(int studentIndex, int mark) => students[studentIndex].SetFinalMark(mark);

    /// <summary>
    /// Sets the answer of the student at the current index
    /// </summary>
    /// <param name="studentIndex">The index of the student</param>
    /// <param name="questionNumber">The question number</param>
    /// <param name="answer">The answer for the student</param>
    public void SetStudentAnswer(int studentIndex, int questionNumber, int answer) => students[studentIndex].SetAnswer(questionNumber, answer);

    /// <summary>
    /// Adds a new answer to the list of the student's answers
    /// </summary>
    /// <param name="studentIndex">The index of the student</param>
    /// <param name="answer">The answer to add</param>
    public void AddStudentAnswer(int studentIndex, int answer) => students[answer].AddAnswer(answer);

    /// <summary>
    /// Gets the text for the question
    /// </summary>
    /// <param name="questionNumber">The question number</param>
    /// <returns>A string of the question's text</returns>
    public string GetQuestionText(int questionNumber) => test.GetQuestionText(questionNumber);

    /// <summary>
    /// Fetches the choice of the question at the index
    /// </summary>
    /// <param name="choice">The index of the choice</param>
    /// <param name="questionNumber">The question number</param>
    /// <returns></returns>
    public string GetQuestionChoice(int choice, int questionNumber) => test.GetChoice(choice, questionNumber);


    /// <summary>
    /// Gets the answer to the question
    /// </summary>
    /// <param name="questionNumber">The question number</param>
    /// <returns>The answer to the question</returns>
    public int GetAnswer(int questionNumber)
    {
        return test.GetAnswerIndex(questionNumber);
    }

    /// <summary>
    /// Saves the students to file
    /// </summary>
    public void SaveStudents()
    {
        fileManager.SaveStudents(students);
    }
}

The FileManger class just loads and saves objects to a file. Any help or guidance would be appreciated!

Answer 1

Student

• The finalMark can be made a public property, then you don't need the getters and setters.
• answers has a few issues. Your implementation limits the external code to using it as an array. There is no way to request how many answers are in the list. If the external code asks for answer 0,1,2,3,... it will eventually run into an IndexOutOfRangeException. There's no way to prevent this.

Simplify the class:

public class Student : User
{
    public int FinalMark { get; set; }
    public List<int> Answers { get; private set; }

    public Student()
    {
        Answers = new List<int>();
    }
}

This works exactly like your class, with the added benefit of avoiding the IndexOutOfRangeException and allowing external callers to use the Answers list however they want (as a list, as an array, in a foreach, ...)

I'm also not a fan of relating answers to students. I would redesign this:

• A test has a student or a student has a test (whichever you prefer)
• A test has questions.
• Every question holds the student's answer (as a property) or the test contains a mapping between questions and given answers (whichever you prefer).

Since the Question contains the correct answer, I would argue that the given answer needs to be stored there too, so you can very easily check if the correct answer was given.

This would be a better OOP approach.

Also:

• It's a bit jarring that you use an index to refer to an answer, and use this index outside of the question class. That index is meaningless without knowing the answer it belongs to.

Redesigning the answers to belong to the questions would solve this issue.

---

MultipleChoiceQuestion

• question and Question can be condensed into a single public Question property.
• Maybe it suits your current needs, but I'd avoid using a hardcoded 4 as your array size. You used a List<int> in your Student class specifically so you could grow the list. I would do the same here.
• Similar to answers in the Student class, you should remove the public properties and instead make the Choices a public List<string> property to avoid things like ArgumentOutOfRangeException and allowing your answers to be used in a foreach.
• Maybe unnecessary for your current needs, but you could redesign the code to allow for multuple correct answers.
• I would rename Answer to CorrectAnswer, in order to disambiguate correct answers from given answers.

Simplify the class:

public class MultipleChoiceQuestion
{
    Public string Question { get; set; }

    public List<string> Choices { get; private set; }

    public int CorrectAnswer { get; set; }

    public MultipleChoiceQuestion()
    {
        Choices = new List<string>();
    }
}

---

Test

• Same remarks as before, don't always make private fields and public properties. If an external caller has access to both setting and getting the value, and you are not doing any extra work when a value is retrieved/set; then a public property suffices.
• List<MultipleChoiceQuestion> question should be called questions because it contains more than one question.
• I would add the Student here as a property.

This seems to be better from an OOP point of view. A Test would then be an object that contains:

• The student that takes the test.
• The questions that are asked on the test.
• The answers that the student has given.

Whether you store the given answer in the test, or in the question itself, is a matter of preference. I would put it in the question but I concede that this may be a subjective preference.

---

StudentController

Nothing out of the ordinary here. You'll need to update the code according to the previously suggested improvements.

One thing though:

public int GetStudentIndex(int ID)
{
    int id = -1;
    foreach (Student student in students)
    {
        if (student.ID == ID)
        {
            id = ID;
            break;
        }
    }
    return id;
}

There is a massive difference between the index of the student, and his ID.

Suppose I have deleted students with ID 4 and 5. Your array of student looks like this:

0,1,2,3,6

The student with ID 6 has index 4! As it stands, your current code will return the same value that you passed into it, which is somewhat pointless.
The only saving grace here is that -1 indicates that the student doesn't exist. However, if that is your only intention, you should redesign the method to be public bool StudentExists(int ID) to make its intention clear.

This is part of a larger issue where you've relied on indexes much more than is common in code nowadays. However, as I don't know how far you've gotten in your curriculum, I'm not sure if you should already be doing this better (or if you still have to learn this in a later class).

What you should realize now, though, is that an index and ID are two very different things:

• An index is a position in an array. A student's index is influenced by how many students are in the array before him.
• An ID is a student's personal identification. This value will never change based on where the student is located in a list of students.

---

Some advanced tips

These are probably out of scope for the current assignment, but they are things I would change if this was a professional project:

• Instead of storing a finalMark value, you could calculate it on the fly. If you set up your classes correctly, you could e.g. calculate it as int finalMark = myTest.Questions.Count(q => q.GivenAnswer == q.CorrectAnswer);
• As mentioned before, I would redesign the question to allow for multiple correct answers.
• For more advanced tests, you'd want to use a Weight attribute. E.g. Question 1 is worth 5 points => Weight = 5. This helps you create tests where some answers are more important than others.

Answer 2

Confusing to me

Student has answers but they do not seem to be associated with a test.

Answers should be specific to a test. A student either has multiple tests or a test has a user. You can have a base class test that is the raw test and then a testUser class that inherits from test.

MultipleChoiceQuestion should have the correct answer so it can be scored.

Test should have a score method.

Use public properties over the get set methods.

You don't need a set on List. They can get then add, revise and delete.

You can just new like this and keep it out of the ctor

private List<int> answers = new List<int>();  

C# has indexer. Using Indexers (C# Programming Guide)