Test/Quiz program to practice OOP

Question

I wanted to practice OOP when writing a program so I decided to make a simple test/quiz. I would appreciate any corrections and improvements to make my code cleaner/shorter/more extensible/more efficient/better naming/more loosely coupled, because right now I feel like my program works but it's tightly coupled and frankly looks horrible (and extremely long...).

program.cs

public class Program
    {
        public static void Main()
        {
            var test = new UserInterface();
            test.RunHowGoodAreYouTest();
        }
    }

UserInterface.cs

public class UserInterface
    {
        public void RunHowGoodAreYouTest()
        {
            var random = new Random();
            var possibleQuestions = Enumerable.Range(0, TestGenerator.TotalQuestions).ToList();
            var questionsLeft = TestGenerator.TotalQuestions;
            var points = 0;

            TestGenerator.DisplayIntro();

            while (true)
            {
                var randomQuestionIndex = RandomQuestionGenerator.GenerateRandomQuestion(random, possibleQuestions);
                var currentQuestion = TestGenerator.Questions[randomQuestionIndex];

                TestGenerator.DisplayCurrentQuestion(currentQuestion);

                var chosenAnswer = Console.ReadLine();
                while (!Utilities.IsValidInput(chosenAnswer))
                {
                    Utilities.DisplayMessage("\nError. Please type A, B, C or D. Or type 'exit' to quit the test.\n");
                    chosenAnswer = Console.ReadLine();
                }
                if (Utilities.ExitTest(chosenAnswer))
                    break;

                var chosenAnswerId = Utilities.TranslateAnswer(chosenAnswer);
                points = PointsSum.SumPointsFromInput(points, currentQuestion.Question.Id, chosenAnswerId);

                if (questionsLeft == 1)
                    break;

                questionsLeft--;
            }

            PointsCalculation.CalculatePointsResult(points);
        }
    }

RandomQuestionGenerator.cs

public static class RandomQuestionGenerator
    {
        public static int GenerateRandomQuestion(Random random, List<int> possibleQuestions)
        {
            var randomNum = random.Next(0, possibleQuestions.Count);
            var randomQuestionIndex = possibleQuestions[randomNum];
            possibleQuestions.RemoveAt(randomNum);
            return randomQuestionIndex;
        }
    }

question.cs

public class Question
    {
        public Question(int id, string title)
        {
            Id = id;
            Title = title;
        }

        public int Id { get; set; }
        public string Title { get; set; }
    }

answer.cs

public class Answer
    {
        public int Id { get; set; }
        public string Title { get; set; }
        public int Points { get; set; }

        public Answer(int id, string title, int points)
        {
            Id = id;
            Title = title;
            Points = points;
        }
    }

QuestionAndAnswers.cs

public class QuestionAndAnswers
    {
        public QuestionAndAnswers(Question question, Answer answerA, Answer answerB, Answer answerC, Answer answerD)
        {
            Question = question;
            AnswerA = answerA;
            AnswerB = answerB;
            AnswerC = answerC;
            AnswerD = answerD;
        }

        public Question Question { get; set; }
        public Answer AnswerA { get; set; }
        public Answer AnswerB { get; set; }
        public Answer AnswerC { get; set; }
        public Answer AnswerD { get; set; }
    }

PointsSum.cs

public static class PointsSum
    {
        public static int SumPointsFromInput(int points, int currentQuestionId, int chosenAnswerId)
        {
            var currentQuestion = TestGenerator.Questions.Find(q => q.Question.Id == currentQuestionId);

            if (currentQuestion.AnswerA.Id == chosenAnswerId)
                points += currentQuestion.AnswerA.Points;
            else if (currentQuestion.AnswerB.Id == chosenAnswerId)
                points += currentQuestion.AnswerB.Points;
            else if (currentQuestion.AnswerC.Id == chosenAnswerId)
                points += currentQuestion.AnswerC.Points;
            else if (currentQuestion.AnswerD.Id == chosenAnswerId)
                points += currentQuestion.AnswerD.Points;

            return points;
        }
    }

PointsCalculation.cs

public static class PointsCalculation
    {
        private static readonly int NotGoodLimit =
            (int) Math.Floor(TestGenerator.TotalQuestions * TestGenerator.NumberOfAnswersPerQuestion / 3.0);

        private static readonly int GoodLimit =
            (int) Math.Floor(TestGenerator.TotalQuestions * TestGenerator.NumberOfAnswersPerQuestion / 2.0);

        private static readonly int VeryGoodLimit =
            (int) Math.Floor(TestGenerator.TotalQuestions * TestGenerator.NumberOfAnswersPerQuestion / 1.3);

        private static readonly int SaintLimit =
            TestGenerator.TotalQuestions * TestGenerator.NumberOfAnswersPerQuestion;

        public static void CalculatePointsResult(int points)
        {
            if (points <= NotGoodLimit)
                Utilities.DisplayMessage("\nYou got " + points + " points out of " + SaintLimit +
                                               ". \nYou are quite evil aren't you?\n");
            else if (points <= GoodLimit)
                Utilities.DisplayMessage("\nYou got " + points + " points out of " + SaintLimit +
                                               ". \nYou are borderline good.\n");
            else if (points <= VeryGoodLimit)
                Utilities.DisplayMessage("\nYou got " + points + " points out of " + SaintLimit +
                                               ". \nYou are a good person.\n");
            else if (points <= SaintLimit)
                Utilities.DisplayMessage("\nYou got " + points + " points out of " + SaintLimit +
                                               ". \nYou are pretty much a Saint. Congrats!!\n");
        }
    }

Utilities.cs

public class Utilities
    {
        public static void DisplayMessage(string message)
        {
            Console.WriteLine(message);
        }

        public static bool ExitTest(string input)
        {
            return input.ToLower().Equals("exit");
        }

        public static bool IsValidInput(object input)
        {
            if (!(input is string))
                return false;
            var strInput = (string) input;
            strInput = strInput.Trim().ToUpper();

            switch (strInput)
            {
                case "A":
                case "B":
                case "C":
                case "D":
                case "EXIT":
                    return true;
                default:
                    return false;
            }
        }

        public static int TranslateAnswer(string choice)
        {
            switch (choice.Trim().ToUpper())
            {
                case "A":
                    return 1;
                case "B":
                    return 2;
                case "C":
                    return 3;
                case "D":
                    return 4;
                default:
                    return -1;
            }
        }
    }

TestGenerator.cs

public static class TestGenerator
    {
        public static readonly List<QuestionAndAnswers> Questions = new List<QuestionAndAnswers>();
        public static readonly int TotalQuestions;
        public static readonly int NumberOfAnswersPerQuestion = 4;

        public const int QuestionIdHomelessMan = 1;

        public const int AnswerIdHomelessManA = 1;
        public const int AnswerIdHomelessManB = 2;
        public const int AnswerIdHomelessManC = 3;
        public const int AnswerIdHomelessManD = 4;

        public const int QuestionIdLittleSibling = 2;

        public const int AnswerIdLittleSiblingA = 1;
        public const int AnswerIdLittleSiblingB = 2;
        public const int AnswerIdLittleSiblingC = 3;
        public const int AnswerIdLittleSiblingD = 4;

        public const int QuestionIdFriendInJail = 3;

        public const int AnswerIdFriendInJailA = 1;
        public const int AnswerIdFriendInJailB = 2;
        public const int AnswerIdFriendInJailC = 3;
        public const int AnswerIdFriendInJailD = 4;

        public const int QuestionIdOldLady = 4;

        public const int AnswerIdOldLadyA = 1;
        public const int AnswerIdOldLadyB = 2;
        public const int AnswerIdOldLadyC = 3;
        public const int AnswerIdOldLadyD = 4;

        static TestGenerator()
        {
            GenerateQuestions();
            TotalQuestions = Questions.Count;
        }

        private static void GenerateQuestions()
        {
            var questionHomelessMan = new Question(QuestionIdHomelessMan,
                "You see a homeless man begging on the street. What do you do?");

            var answerHomelessManA = new Answer(AnswerIdHomelessManA, "I give him some money.", 2);
            var answerHomelessManB = new Answer(AnswerIdHomelessManB, "I buy him a sandwich.", 3);
            var answerHomelessManC = new Answer(AnswerIdHomelessManC, "I just walk past him.", 1);
            var answerHomelessManD = new Answer(AnswerIdHomelessManD, "I take him into my home.", 4);
            var questionAndAnswersHomelessMan = new QuestionAndAnswers(questionHomelessMan, answerHomelessManA,
                answerHomelessManB, answerHomelessManC, answerHomelessManD);

            Questions.Add(questionAndAnswersHomelessMan);

            var questionLittleSibling = new Question(QuestionIdLittleSibling,
                "Your little sibling wants to play with you but you're too busy right now. What do you do?");

            var answerLittleSiblingA = new Answer(AnswerIdLittleSiblingA, "You play with him for while.", 4);
            var answerLittleSiblingB = new Answer(AnswerIdLittleSiblingB, "You tell him to get the fuck out.", 1);
            var answerLittleSiblingC = new Answer(AnswerIdLittleSiblingC,
                "You tell him to go play with another family member.", 2);
            var answerLittleSiblingD = new Answer(AnswerIdLittleSiblingD,
                "You tell him that you will play with him later.", 3);
            var questionAndAnswersLittleSibling = new QuestionAndAnswers(questionLittleSibling, answerLittleSiblingA,
                answerLittleSiblingB, answerLittleSiblingC, answerLittleSiblingD);

            Questions.Add(questionAndAnswersLittleSibling);

            var questionFriendInJail = new Question(QuestionIdFriendInJail,
                "Your friend has committed a crime and is now in jail. What do you do?");

            var answerFriendInJailA = new Answer(AnswerIdFriendInJailA, "You give him a call if it's allowed.", 2);
            var answerFriendInJailB = new Answer(AnswerIdFriendInJailB, "You want nothing to do with him anymore.", 1);
            var answerFriendInJailC = new Answer(AnswerIdFriendInJailC,
                "You may visit him in jail sometime next month.", 3);
            var answerFriendInJailD = new Answer(AnswerIdFriendInJailD, "You visit him in jail as soon as possible.",
                4);
            var questionAndAnswersFriendInJail = new QuestionAndAnswers(questionFriendInJail, answerFriendInJailA,
                answerFriendInJailB, answerFriendInJailC, answerFriendInJailD);

            Questions.Add(questionAndAnswersFriendInJail);

            var questionOldLady = new Question(QuestionIdOldLady,
                "This lonely old lady is happily telling you all about her life while you're at the bus stop. What do you do?");

            var answerOldLadyA = new Answer(AnswerIdOldLadyA, "You kindly tell her to shut the fuck up.", 1);
            var answerOldLadyB = new Answer(AnswerIdOldLadyB,
                "You make up a lie (eg you need to phone someone) to get rid of her.", 2);
            var answerOldLadyC = new Answer(AnswerIdOldLadyC, "You actively listen to her.", 4);
            var answerOldLadyD = new Answer(AnswerIdOldLadyD, "You try to listen to her but do not really engage.", 3);
            var questionAndAnswersOldLady = new QuestionAndAnswers(questionOldLady, answerOldLadyA,
                answerOldLadyB, answerOldLadyC, answerOldLadyD);

            Questions.Add(questionAndAnswersOldLady);
        }

        public static void DisplayIntro()
        {
            Utilities.DisplayMessage("\n***** HOW GOOD ARE YOU? *****\n");
        }

        public static void DisplayCurrentQuestion(QuestionAndAnswers currentQuestion)
        {
            Utilities.DisplayMessage("");
            Utilities.DisplayMessage(currentQuestion.Question.Title + "\n");

            Utilities.DisplayMessage("A) " + currentQuestion.AnswerA.Title);
            Utilities.DisplayMessage("B) " + currentQuestion.AnswerB.Title);
            Utilities.DisplayMessage("C) " + currentQuestion.AnswerC.Title);
            Utilities.DisplayMessage("D) " + currentQuestion.AnswerD.Title);

            Utilities.DisplayMessage("\nChoose A, B, C or D. Or type 'exit' to quit the test.");
        }
    }

Answer 1

This is your requirement:

it's a sort of personality test. It displays on the console a random question with 4 possible answers, and the user chooses one by typing a b c or d. Or they can type 'exit' at any time to exit the console. Once the user has answered all the questions, they get a result based on the points that each answer has (1-4). So in this code example there are 4 questions with 4 answers each with scores 1-4. So the max score someone can possibly get is 16 and the minimum is 4. The result message depends on the points they get at the end.

One of the idea behind OOP is to make your code easier to understand, easier to test and self encapsulated. In order to do that, we should aim to introduce concepts that will be used so the program is easier to understand and not error prone. The concepts should be within the context of the domain of the program. You have, sort of, done this. But we need to go a step further and I will explain what I mean by that soon.

Ok let's start it step by step.

it's a sort of personality test

Ok! In that case it would be nice if we have a class called PersonalityTest.

It displays on the console a random question with 4 possible answers, and the user chooses one by typing a b c or d.

Ok, so the question has 4 different answers. We need a class called Question and you already have this class. But remember I said we need to go a step further, well here is what I meant. Whenever, you write a class, ask yourself these questions:

1. Does the name of the class clearly indicate the concept it is abstracting?
2. Does the public interface of the class clearly manifest it's characteristics and behavior?
3. This one is important: Can a developer put the class into an invalid state at any point and break the system?

You have the Question class:

public class Question
{
    public Question(int id, string title)
    {
        Id = id;
        Title = title;
    }

    public int Id { get; set; }
    public string Title { get; set; }
}

For the current domain, personality test, this class is not good because after the question is created, the developer can modify it. For example,

var q1 = new Question(1, "Title");
// then somewhere the developer can do this
q1.Id = -10000; // or something else
q1.Title = "l;kjlkkjs09809809slkj";

Now you may ask why would the developer do that? The answer to that is simple: Because you are allowing it. But a better answer is: Oops I did not know I was not supposed to do that. Either way, it is a bug. But in a large scale application, this is what good developers do: they look at the interface and then program against it. Imagine, .NET developers had a public setter for the Count property of list, you put 100 items in it, and then set the Count to zero. That would be disaster. Ok you get the point. Let's make it better.

A question has 4 possible answers so let's make sure that is also in the public interface of the Question class. Please read the comments within the code.

public class Question
{
    // Let's make this a constant so if we change our mind in the future,
    // the change is needed in one place
    private const int AllowedNumOfAnswers = 4;

    // A question has answers. Once the question is created, you cannot
    // change the answers.

    // Why IEnumerable<Answer> and not List<Answer> or something else?
    // Because we do not intend to add anything to the list of answers.
    // This makes our intention clear and the interface is clear to the 
    // developer using our class.
    public Question(int id, string title, IEnumerable<Answer> answers)
    {
        // How about your business rule of 4 answers only?
        if (answers == null || answers.Count() != AllowedNumOfAnswers)
        {
            throw new InvalidOperationException(
                $"A question must have exactly { AllowedNumOfAnswers } answers");
        }
        Id = id;
        Title = title;
    }

    // Setters are private. Once the object is instantiated, it cannot be
    // changed.
    public int Id { get; private set; }
    public string Title { get; private set; }

    // Again why IEnumerable? Well same answer because we want to make our 
    // intention clear that this class will provide answers that clients
    // (users of the class i.e. developers) can iterate, but they cannot
    // add new answers to it.
    // Notice the private set
    public IEnumerable<Answer> Answers { get; private set; }

    // I will leave this to you. The idea is this can be set to id of 
    // one of the answers for this question. Make sure the id exists in 
    // the answer to these questions
    public int ChoosenAnswerId { get; set;}
}

Let's do the same for Answer:

public class Answer
{
    private const int MinPoint = 1;
    private const int MaxPoint = 4;
    public int Id { get; private set; }
    public string Title { get; private set; }
    public int Points { get; private set; }

    public Answer(int id, string title, int points)
    {
        // We have a business rule
        if (MinPoint > points || points > MaxPoint)
        {
            //...
        }
        Id = id;
        Title = title;
        Points = points;
    }
}

Great, now we have solid classes that cannot be put into a bad state. Plus it makes our intention very clear that the classes are immutable.

If we were to do this personality test in real life, we may have someone that asks us the questions. That person will have all the questions. It would be odd if this person asked us the same question more than once, so let's take that into consideration.

Let's call this concept a Moderator or a Questioner. I will go with Moderator.

public class Moderator 
{
    private Random random = new Random();

    // It would be better if this is stack. You keep popping questions from
    // it.
    private IList<Question> internalQuestions;

    public Moderator(IEnumerable<Question> questions)
    {
        if (questions == null)
        {
            // No good if it is null.
            throw new InvalidOperationException("...");
        }
        var unique = questions.GroupBy(x => x.Title).Count() == 1;
        var idsUnique = questions.GroupBy(x => x.Id).Count() == 1;
        if (!unique || !idsUnique)
        {
            // ...
        }

        this.Questions = questions;
        // We will need to index the questions
        this.internalQuestions = questions.ToList();
    }

    // Returns the next question or null if there are no more questions
    public Question Next()
    {
        if (this.internalQuestions.Count == 0)
        {
            // No more questions
            return null;
        }
        var randomNum = random.Next(0, this.internalQuestions.Count);
        var nextQuestion = this.internalQuestions[randomNum];
        this.internalQuestions.RemoveAt(randomNum);

        return nextQuestion;
    }

    public IEnumerable<Question> Questions { get; private set; }
}

---

Make Reusable Classes

Or they can type 'exit' at any time to exit the console.

That is fine. However, none of your classes should be aware of that. In other words, if I wanted to use your class in a windows forms application, it would be nice if it also worked there. Therefore, I would remove this from your Utilities class so the class is not tied to the console:

public static void DisplayMessage(string message)
{
    Console.WriteLine(message);
}

Now the Utilities class can be used anywhere.

---

Few Words on Constants

I do not think you will need all the constants in TestGenerator such as:

public const int AnswerIdHomelessManA = 1;

If you are using something in only 1 place and making it a constant provides no additional meaning, then you do not need to make it a constant (it already is a constant value). If making it a constant provides additional meaning, then yes make it a const. For example:

if (age < 18) // what is so special about 18?

then if we do this, it provides additional meaning:

const int LegalAge = 18;
if (age < LegalAge) // has meaning

I do not see the case for AnswerIdHomelessManA giving additional meaning but I could be wrong and leave the decision up to you.

As soon as you use it in more than 1 place, you can then make it a constant. Try to start at the lowest point of exposure and work your way up: for example, if you needed AnswerIdHomelessManA in multiple places within a method, then make it a const within the method (not within the class). If you need it within multiple methods, then make it within the class. If multiple classes, then within the namespace. If multiple namespaces, within the assembly.

---

Use the right Loop

There are many loops available. You used while loop. However, that is not the best choice. If you know you will do something at least once and/or possibly more times, then use the do while construct. For example, you know you will show at least one question or the message "no questions available" at least once, so the do while is the right tool.

---

Let's put it all together

Let's create the PersonalityTest class:

public class PersonalityTest
{
    private Moderator moderator;
    // Even in the real world, we could not start the test without a moderator
    public void Start(Moderator moderator)
    {
        if (moderator == null)
        {
            //...
        }

        this.moderator = moderator;
    }

    public int GetTotalScore()
    {
        ScoreCalculator.Calculat(this.moderator.Questions);
    }
}

You may ask: Why is the PersonalityTest class using the moderator.Questions property and why did we not give the PersonalityTest the list of questions? The answer: We need to make sure when the PersonalityTest class is asked for the score, it uses the same list of questions that the moderator asked. Otherwise, it will be easy to have bugs in the system. Developers using our classes may create one list and give it to the moderator. Then create another list and give it to the PersonalityTest.

Let's use it:

PersonalityTest test = new PersonalityTest();
var questions = new List<Question>();
// fill the questions list
var moderator = new Moderator(questions);
test.Start(moderator);

bool thereAreMoreQuestions = true;
bool userHasDecidedToExit = false;
do
{
    var next = moderator.Next();
    if (next == null)
    {
        thereAreMoreQuestions = false;
    }
    else
    {
        bool isValidInput = false;
        do
        {
            Utilities.DisplayMessage("\nError. Please type A, B, C or D. Or type 'exit' to quit the test.\n");
            var input = Console.ReadLine();
            isValidInput = !Utilities.IsValidInput(input);
            userHasDecidedToExit = Utilities.ExitTest(input);
            if (isValidInput && !userHasDecidedToExit)
            {
                next.ChosenAnswerId = 
                    Utilities.TranslateAnswer(input);
            }
        } while(!userHasDecidedToExit && !isValidInput)
    }
} while (thereAreMoreQuestions && !userHasDecidedToExit);

int total = test.GetTotalScore();

You may get curious and ask why I did this in the above class:

if (next == null)
{
    noMoreQuestions = true;
}
else
{
}

and not this:

if (next != null)
{
}
else
{
    noMoreQuestions = true;
}

The reason: I always try to write optimistic conditions. I find they are a lot more natural and easier to understand. In the real world we do not say: All the people who are not over 18, do not go to room A. We say: All the people who are over 18, go to room A. Therefore, I try to write code with the same mentality.

There are more things you can improve, for example:

Utilities.ExitTest(input)

is not a good method name because it seems like you are asking it to exit the test. But you are actually asking it if the input entered is to exit the test. It should be:

Utilities.IsInputForExit(input) Or IsInputToExit

---

In Conclusion

Our program has these classes:

PersonalityTest
Question
Answer
Moderator
ScoreCalculator
// and your utility classes

If anyone sees the class diagram with the above classes, they will get a clear picture of what the program is about. Also, we have made it for hard for developers who will use these classes and plug them into each other to make innocent mistakes.