4
\$\begingroup\$

I'm currently attending ERAU (online), and in my Computing for Engineers class we are learning C#. An ongoing project throughout the class is to build a "quiz" program that asks 10 questions, in order, and then repeats missed questions (in order) on a second iteration. There are 2 types: T/F and multiple choice. 4 T/F and 6 mc; the mc questions can have 4 possible answers. The program must keep track of the number of each type of question that was answered correctly and display that at the end.

I'm just learning about loops, conditional statements, arrays and methods (last week), and here's the code I came up with (it compiles and runs, and does exactly what I want it to do, but my instructor said it is too long and not optimized):

using System;
using System.Threading;

namespace Quiz
{
    class Program
    {
        static void Main(string[] args)
        {
            //start the quiz by telling the student the details of the structure of the quiz and the subject being tested
            Console.WriteLine("Kenneth Myers, ENGR115, Assignment: Checkpoint 4 & 5");
            Console.WriteLine("You are about to take a 10 question quiz on Binary Number Systems.");
            Console.WriteLine("Each question will be displayed with either a True/False or Multiple-choice answer key. Select the correct answer by pressing the corresponding key.");
            Console.WriteLine("If you get the question wrong, you will be notified of the wrong answer and the next question will be displayed.");
            Console.WriteLine("Once all questions have been displayed once, questions answered incorrectly will be displayed in the original order; you may attempt the question again.");
            Console.WriteLine("After all missed questions have been displayed a second time and answered, the results of your quiz will be displayed, by category of question and number answered correctly, and the total number answered correctly.");
            Thread.Sleep(5000);//adds a 5 second delay before the next operation to give the user time to read the instructions
            Console.Clear(); //clears the screen before starting the test
            //create an array with all the questions and possible answers to display to the user
            var questions = new string[] { "Question 1: True or False – Binary numbers consist of a sequence of ones and zeros? Press T or F and enter to submit", "Question 2: Multiple choice – Add the following binary numbers: 1011+0011. Answers: 1) 1110, 2) 1022, 3) 1010, 4) 1032. Select the number (1,2,3 or 4) of the correct answer and enter to submit", "Question 3: Multiple choice – Add the following binary numbers: 11011011+01100110. Answers: 1) 11000001, 2) 11111111, 3) 12111121, 4) 101000001. Select the number (1,2,3 or 4) of the correct answer and enter to submit", "Question 4: True or False – Binary numbers are base-10? Press T or F and enter to submit", "Question 5: Multiple choice – Subtract the following binary numbers: 1110-0101. Answers: 1) 1010, 2) 1001, 3) 1000, 4) 0100. Select the number (1,2,3 or 4) of the correct answer and enter to submit", "Question 6: Multiple choice – Subtract the following binary numbers: 10010011-01101100. Answers: 1) 01101111, 2) 00010111, 3) 00100111, 4) 11011101. Select the number (1,2,3 or 4) of the correct answer and enter to submit", "Question 7: True or False – Binary numbers are base-2? Press T or F and enter to submit", "Question 8: Multiple choice – the binary number 1011 equates to what base-10 number? Answers: 1) 11, 2) 22, 3) 14, 4) 7. Select the number (1,2,3 or 4) of the correct answer and enter to submit", "Question 9: Multiple choice – what is the binary equivalent of the base-10 number 127? Answers: 1) 01101111, 2) 11111111, 3) 10111011, 4) 11111110. Select the number (1,2,3 or 4) of the correct answer and enter to submit", "Question 10: True or False: an 8-bit binary number can have a maximum value of 128 in base-10? Press T or F and enter to submit" };
            var questionAnswer = new string[] { "t", "1", "4", "f", "2", "3", "t", "1", "2", "f" };//creating array of correct answers
            var questionInput = new string[10];//create the input variables for each question to accept user input in an array
            var questionStatus = new string[] { "n", "n", "n", "n", "n", "n", "n", "n", "n", "n" };//create question status array to track if the question was answered correctly the first time
            var questionType = new string[] { "tf", "mc", "mc", "tf", "mc", "mc", "tf", "mc", "mc", "tf" };//define the question types (tf or mc) in an array
            var questionIteration = 0;//set the question iteration globally
            var questionCount = 0;//set the question count globally
            var trueFalseCount = 0; var multipleChoiceCount = 0; //sets the true/false counter and multiple choice counters to zero
            //start the quiz - display each question, in order starting at question 1, and accept the user input for that question, then display the user input and move to the next question
                Console.Clear();//clear the screen
                for (questionIteration = 1; questionIteration < 3; questionIteration++)//set up a for loop to run 2 iterations of the questions
                {
                    questionCount = 0;
                    if (questionIteration == 2)
                {
                    Console.WriteLine("Second attempt. Previously incorrectly answered questions will be displayed again. Good luck!");
                }
                    foreach (string question in questions)//foreach loop to handle each question and the answer input by the user, plus logic to test for invalid and correct/incorrect answers
                    {
                        if (questionStatus[questionCount] == "n")//first attempt at the question
                        {
                            Console.WriteLine(question); //displays the question, starting with question 1
                            if (questionType[questionCount] == "tf")//logic for true/false questions
                            {
                                questionInput[questionCount] = InvalidTrueFalseEntry(); //accepts user input for answer to question and stores in variable; later, passes input to method to test if valid input type
                                questionInput[questionCount] = CheckForCorrectAnswer();//checks for correct answer
                            }
                            else if (questionType[questionCount] == "mc")//logic for multiple choice questions
                            {
                                questionInput[questionCount] = InvalidMultipleChoiceEntry(); //accepts user input for answer to question and stores in variable; later, passes input to method to test if valid input type
                                questionInput[questionCount] = CheckForCorrectAnswer();//checks for correct answer
                            }
                            Console.Clear();//clear the screen
                        }
                        //else if (questionStatus[questionCount] == "y")
                        //{
                        //Console.WriteLine("Second attempt!");//tell the user it's their second attempt
                        //}
                        questionCount++;//increment the question counter for the next iteration
                    }
                }
            Console.WriteLine("Test Complete!");//tells user the test is over
            Console.WriteLine("You answered " + trueFalseCount + " T/F questions correctly and " + multipleChoiceCount + " multiple choice questions correctly.");//tells user their performance
            Console.WriteLine("You missed " + (4 - trueFalseCount) + " T/F questions and " + (6 - multipleChoiceCount) + " multiple choice questions.");
            Thread.Sleep(5000); Console.Clear();//clear the screen
            string InvalidMultipleChoiceEntry()//this method will check multiple choice answers to ensure they are a number 1-4
            {
                var answer = Console.ReadLine();
                if (answer != "1" && answer != "2" && answer != "3" && answer != "4")//did the user input a numner between 1 and 4?
                {
                    Console.WriteLine("Invalid answer type (not 1-4), try again.");//if not, try again until they do!
                    return InvalidMultipleChoiceEntry();
                }
                return answer;
            }
            string InvalidTrueFalseEntry()//this method will check true/false answers to ensure input was a "t" or an "f"
            {
                var answer = Console.ReadLine();//get the user input
                if (answer != "t" && answer != "f")//did the user input a t or f?
                {
                    Console.WriteLine("Invalid answer type (not t or f), try again.");//if not, try again until they do!
                    return InvalidTrueFalseEntry();
                }
                return answer;
            }
            string CheckForCorrectAnswer()//this method will check the answer to see if it is correct or not, if the entry was valid for the questionType
            {
                var answer = questionInput[questionCount];
                if (answer != questionAnswer[questionCount])//tests for incorrect answer given from a valid input by question type
                {
                    Console.WriteLine("Your answer was: " + questionInput[questionCount]); //displays the answer chosen by the user
                    Console.WriteLine("Sorry, wrong answer. :-(");
                    Thread.Sleep(2000);
                    questionStatus[questionCount] = "n";//make sure the question status reflects "n"
                    return answer;
                }
                else //it must be correct, since we've checked against invalid and incorrect answers!
                {
                    Console.WriteLine("Your answer was: " + questionInput[questionCount]); //displays the answer chosen by the user
                    Console.WriteLine("Correct answer!");
                    questionStatus[questionCount] = "y";
                    if (questionType[questionCount] == "tf")
                    {
                        trueFalseCount++;//increment the true/false counter to total the number answered correctly
                    }
                    else
                    {
                        multipleChoiceCount++;//increment the multiple choice counter to total the number answered correctly
                    }
                    Thread.Sleep(2000);
                    return answer; 
                }
            }
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Have they taught you how to write your own functions yet? How much do you know about passing arguments? You say you're learning OOP, but I don't see much of it in your code. \$\endgroup\$ – Mast Feb 8 at 15:14
  • \$\begingroup\$ I'm sorry, let me clarify: this week we are starting the chapter on OOP. We have not implemented anything in our code yet, but we are learning about the 4 fundamentals of OOP and creating and using objects. As for functions, I don't recall learning functions, unless you mean methods, which according to my textbook are called "functions" in other programming languages. I assume that is what you mean, and yes, we went over methods last week. I'm still struggling with methods right now. \$\endgroup\$ – MX372 Feb 8 at 17:43
  • \$\begingroup\$ C# still has functions, but methods are special in that they are functions belonging to a class/object. \$\endgroup\$ – Mast Feb 8 at 17:49
  • \$\begingroup\$ I've re-written the code I had posted originally, and it is more efficient and works as I intended, but now I need to do this using OOP principles; meaning, separate objects (classes?). This is where I need some help. If I create a class, for example, for the questions (class Questions), would it make sense to define within that class the following: list of questions, whether they have been asked already, and if they have been answered correctly? \$\endgroup\$ – MX372 Feb 10 at 19:46
  • 1
    \$\begingroup\$ Learning path: I'd not create a class for the list of questionS but a class for a single Question (with question, answer and type). Then redo this using a single array of them. Then remove the type field (which should not be a string but an enum) and use derived classes (if you learned about them). Then abstract input logic away into a separate function. Side note: remove those Thread.Sleep()...just leave the text on the screen without Clear() as a favor for your users! \$\endgroup\$ – Adriano Repetti Feb 11 at 9:51
2
\$\begingroup\$

The first thing I noticed about your code is that it is very compressed. There are almost no empty lines, and the lines are quite long.

The longest line in your code is 1745 characters long. Assuming that a single character is 2 mm wide, I'd need a screen of 3.5 meters to show the whole line. I don't have such a screen.

To understand that single line, I had to split it up into this block of code:

//create an array with all the questions and possible answers to display to the user
var questions = new[] {
    "Question 1: True or False – Binary numbers consist of a sequence of ones and zeros? "
    + "Press T or F and enter to submit",

    "Question 2: Multiple choice – Add the following binary numbers: 1011+0011. "
    + "Answers: 1) 1110, 2) 1022, 3) 1010, 4) 1032. "
    + "Select the number (1,2,3 or 4) of the correct answer and enter to submit",

    "Question 3: Multiple choice – Add the following binary numbers: 11011011+01100110. "
    + "Answers: 1) 11000001, 2) 11111111, 3) 12111121, 4) 101000001. "
    + "Select the number (1,2,3 or 4) of the correct answer and enter to submit",

    "Question 4: True or False – Binary numbers are base-10? "
    + "Press T or F and enter to submit",

    "Question 5: Multiple choice – Subtract the following binary numbers: 1110-0101. "
    + "Answers: 1) 1010, 2) 1001, 3) 1000, 4) 0100. "
    + "Select the number (1,2,3 or 4) of the correct answer and enter to submit",

    "Question 6: Multiple choice – Subtract the following binary numbers: 10010011-01101100. "
    + "Answers: 1) 01101111, 2) 00010111, 3) 00100111, 4) 11011101. "
    + "Select the number (1,2,3 or 4) of the correct answer and enter to submit",

    "Question 7: True or False – Binary numbers are base-2? "
    + "Press T or F and enter to submit",

    "Question 8: Multiple choice – the binary number 1011 equates to what base-10 number? "
    + "Answers: 1) 11, 2) 22, 3) 14, 4) 7. "
    + "Select the number (1,2,3 or 4) of the correct answer and enter to submit",

    "Question 9: Multiple choice – what is the binary equivalent of the base-10 number 127? "
    + "Answers: 1) 01101111, 2) 11111111, 3) 10111011, 4) 11111110. "
    + "Select the number (1,2,3 or 4) of the correct answer and enter to submit",

    "Question 10: True or False: an 8-bit binary number can have a maximum value of 128 in base-10? "
    + "Press T or F and enter to submit"
};

I have no idea how you could ever edit all this text in a single line. Having it split into multiple lines has these benefits:

  • readers of your code can see all the questions on one screen
  • the strings are split at natural boundaries (using the + operator)
  • redundant information becomes clearly visible
    • the question number is written explicitly into each question. This means that the questions cannot be shuffled. Any good quiz program must shuffle the questions as much as possible to avoid learning patterns like "the first question is f".
    • each mc question has the same structure: <Question> Answer: 1) <A1>, 2) <A2>, 3) <A3>, 4) <A4>. This structure should not be encoded in a string but in the code of the program. That's something that will hopefully be covered later in class.
    • each tf question ends with the "Press T or F" sentence
    • each mc question ends with the "Select" sentence

These redundancies do not occur in good programs. The redundant text should better be handled when printing the question, in code like this:

if (questionType[questionCount] == "tf") {
    Console.WriteLine("Press T or F and Enter to submit");
} else if (questionType[questionCount] == "mc") {
    Console.WriteLine("Select the number (1, 2, 3 or 4) of the correct answer and press Enter to submit");
}

This brings me directly to the next item, which is the variable questionCount. A count by convention means the total number of something. The count of questions in this program is 10, and this count doesn't change. Therefore the variable name is misleading. It should rather be named questionIndex. By convention, an index is typically between 0 and the corresponding count, which is exactly the case here.

In the very first part of my answer I said that your code looks very compressed. This is because it doesn't contain empty lines in the usual places. Empty lines should mark all places where in traditional writing you would end a paragraph. One of these places is before a function is defined, such as string InvalidMultipleChoiceEntry(). Before each function definition there should be an empty line. The comment describing the function should be in an extra line before the function definition. Like this:

// This method will check multiple choice answers to ensure they are a number 1-4
string InvalidMultipleChoiceEntry() 
{
    var answer = Console.ReadLine();
    if (answer != "1" && answer != "2" && answer != "3" && answer != "4") //did the user input a numner between 1 and 4?
    {
        Console.WriteLine("Invalid answer type (not 1-4), try again."); //if not, try again until they do!
        return InvalidMultipleChoiceEntry();
    }

    return answer;
}

Calling InvalidMultipleChoiceEntry from inside InvalidMultipleChoiceEntry is a dangerous technique in most programming languages. It can lead to unexpected memory consumption. To test this, replace the call Console.ReadLine(); with a simple "invalid" and run the program. It will run for some seconds and then crash with this error message:

Process is terminating due to StackOverflowException.

To avoid this error, rewrite the function like this:

string InvalidMultipleChoiceEntry()
{
    while (true) {
        var answer = Console.ReadLine();
        if (answer == "1" || answer == "2" || answer == "3" || answer == "4") {
            return answer;
        }

        Console.WriteLine("Invalid answer type (not 1-4), try again.");
    }
}

When I ran the program for the first time, the 5 seconds you gave me to read the intro were way too short. Or maybe the intro was too long, who knows. If you remove the Thread.Sleep(5000); and replace the Console.Clear with Console.WriteLine, everything is fine.

There's probably more to say, but on the other hand the above is already enough work for one or two hours.

\$\endgroup\$
  • \$\begingroup\$ @ Roland, Thanks for your advice, example code, and explanations! This has helped me immensely; exactly what I needed. I also re-wrote my code and implemented your suggestions, and also changed the other function for true false questions to be a while loop instead of an if loop. \$\endgroup\$ – MX372 Feb 12 at 21:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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