Problem
Are there any ways I can make my program more efficient? My program runs, but are there any things that are unneeded that I included?
The local Driver’s License Office has asked you to write a program
that grades the written portion of the license exam. The exam has 20
multiple choice questions. Here are the correct answers:1. B 2. D 3. A 4. A 5. C 6. A 7. B 8. A 9. C 10. D 11.B 12. C 13. D 14. A 15. D 16. C 17. C 18. B 19. D 20. AA student must correctly answer 15 questions of the 20 questions to
pass the exam.
Write a class named DriverExam that holds the correct answers to the exam in an array field. The class should have an array field to
hold the student’s answers. The class should have the following
methods:passed: The method returns true if the student passed the exam, false
otherwise totalCorrect: returns the total number of correctly answered
questions totalIncorrect: returns the total number of incorrectly
answered questions questionsMissed: an int array containing the
question numbers of the question that the student missedDemonstrate the class in a test program that asks the user to enter a
student’s answers, and then display the results returned from the
DriverExam class’s methods.
Input validation: only accept the letters A, B, C, or D as answers
public class DriverExam
{
//An array containing a student's answers
private String[] correctAnswers =
{"B", "D", "A", "A", "C", "A",
"B", "A", "C", "D",
"B", "C", "D", "A",
"D", "C", "C", "B", "D", "A"};
//Store the user's answers
private String[] userAnswers;
int[] missed = new int[correctAnswers.length];
//Process the user's answers
public DriverExam (String[] Answers)
{
userAnswers = new String[Answers.length];
for (int i = 0; i < Answers.length; i++)
{
userAnswers[i] = Answers[i];
}
}
//Returns a boolean value if correct answers > 15
public boolean passed()
{
if (totalCorrect() >= 15)
return true;
else
return false;
}
//Determines the total correct answers
public int totalCorrect()
{
int correctCount = 0;
for (int i = 0; i < correctAnswers.length; i++)
{
if (userAnswers[i].equalsIgnoreCase(correctAnswers[i]))
{
correctCount++;
}
}
return correctCount;
}
//Determines the total incorrect answers
public int totalIncorrect()
{
int incorrectCount = 0;
for (int i = 0; i < correctAnswers.length; i++)
{
if (!userAnswers[i].equalsIgnoreCase(correctAnswers[i]))
{
missed[incorrectCount] = i;
incorrectCount++;
}
}
return incorrectCount;
}
//Missed questions
public int[] questionsMissed()
{
return missed;
}
}
//end of DriverExam class
/* The DriverExamApplication class demonstrates the methods of DriverExam class. */
import java.util.Scanner;
public class DriverExamApplication
{
public static void main(String[] args)
{
System.out.println(" Driver's License Exam ");
Scanner input = new Scanner(System.in);
System.out.println(" 20 Multiple-Choice Questions ");
System.out.println(" Mark A, B, C, D ");
//Inputting string
String[] answers = new String[20];
String answer;
for (int i = 0; i < 20; i++)
{
do
{
System.out.print((i+1) + ": ");
answer = input.nextLine();
} while (!isValidAnswer(answer));
answers[i] = answer;
}
//Process
DriverExam exam = new DriverExam(answers);
//Results
System.out.println(" RESULTS ");
//Outputting total correct
System.out.println("Total Correct: " + exam.totalCorrect());
//Outputting total incorrect
System.out.println("Total Incorrect: " + exam.totalIncorrect());
String passed = exam.passed() ? "YES" : "NO";
//Result pass or fail
System.out.println("Passed: " + passed);
if (exam.totalIncorrect() > 0)
{
System.out.println("The incorrect answers are: ");
int missedIndex;
for (int i = 0; i < exam.totalIncorrect(); i++)
{
missedIndex = exam.questionsMissed()[i]+1;
System.out.print(" " + missedIndex);
}
}
} //end of main function
//Returns true when answer is valid
public static boolean isValidAnswer (String answer)
{
return "A".equalsIgnoreCase(answer) ||
"B".equalsIgnoreCase(answer)
|| "C".equalsIgnoreCase(answer) ||
"D".equalsIgnoreCase(answer);
}
} //end of Test class
Solution
I would like to propose a number of refactorings:
I don’t quite understand why the missed array is an int array which stores it’s own indices if the answers are incorrect. A more logical choice would be a boolean array. We should also only calculate it only once, so we might as well do it in the constructer (which also only runs once per answer). The same applies to total number of correct/incorrect answers. The rest of my commentary is in the code.
// this should be static since it can be shared between all instances of the test
private static String[] correctAnswers =
{"B", "D", "A", "A", "C", "A",
"B", "A", "C", "D",
"B", "C", "D", "A",
"D", "C", "C", "B", "D", "A"};
// lets leave initializing to the constructor, and
// let's store the values so we only calculate them once
// Also, make sure they are private to restrict access
private boolean[] missed;
private int correct;
private int incorrect;
private String[] userAnswers;
//Process the user's answers
public DriverExam (String[] answers)
{
missed = new boolean[answers.length];
userAnswers = new String[answers.length];
correct = 0;
incorrect = 0;
for (int i = 0; i < answers.length; i++)
{
userAnswers[i] = answers[i];
missed[i] = userAnswers[i].equalsIgnoreCase(correctAnswers[i])
if (!missed[i]) {
correct++;
} else {
incorrect++;
}
}
}
//Returns a boolean value if correct answers > 15
public boolean passed()
{
return correct >= 15; // don't use if/else when you are using a boolean expression
}
/*
* Let's use the values we calculated to make the methods
* very simple and easy to read. In addition, we only calculate things once
* which makes our code more efficient
*/
public int totalCorrect()
{
return correct;
}
public int totalIncorrect()
{
return incorrect;
}
public boolean[] questionsMissed()
{
return missed;
}
From my comment:
How does a student miss a question if the only valid answers are
A,B,C, andD? I’m assuming that is different from an incorrect answer, since two different terms are being used here.
I’m just going to assume that a missed answer can either be a null value or an empty String.
Also, are you expected not to use any of the Collections classes because of some reasons? I’m asking this as there seems to be a heavy reliance on arrays instead of the Collections framework…
Putting aside the user input validation parts, and focusing solely on your DriverExam class, I will too ‘calculate things once’ as suggested by mleyfman as such:
public class DriverExam {
private static final String[] CORRECT_ANSWERS = { "B", "D", "A", "A", "C", "A", "B", "A", "C", "D", "B", "C", "D", "A", "D", "C", "C", "B", "D", "A" };
private static final int EXPECTED_TOTAL = CORRECT_ANSWERS.length;
private static final int PASSING_MARK = 15; // or would this be better expressed as 75%?
private final int correct;
private final int incorrect;
private final boolean passed;
private final int[] missed;
public DriverExam(final String[] answers) {
if (answers == null || answers.length != EXPECTED_TOTAL) {
throw new IllegalArgumentException("Wrong answers specified.");
}
int correctCount = 0;
final List<Integer> missedCounters = new ArrayList<>();
for (int i = 0; i < EXPECTED_TOTAL; i++) {
if (answers[i] == null || answers[i].isEmpty()) {
missedCounters.add(Integer.valueOf(i));
}
if (answers[i].equalsIgnoreCase(CORRECT_ANSWERS[i])) {
correctCount++;
}
}
correct = correctCount;
missed = new int[missedCounters.size()];
int i = 0;
for (final Integer value : missedCounters) {
missed[i++] = value.intValue();
}
incorrect = EXPECTED_TOTAL - correctCount - missed.length;
passed = correctCount > PASSING_MARK;
}
}
As Java does not allow arrays of an undefined size, I have to decide between using a List to ‘mark’ missed answers, or resort to iterate answers twice – once to count the number of missed answers, and the second time to actually populate the missed[] array. I went with the former to make the implementation slightly easier.
After initializing the values correct, incorrect, passed and missed as such, the methods you are required to implement can simply return those values… I hope this helps you.
The first thing I would do is convert A,B,C, and D to 1,2,3, and 4. Maybe 0-3, not sure yet. yeah 0,1,2,3. 0,1,2,3 just makes so much more sense than A,B,C,D. Very likely down the road it will prove to be a good move.
Do an ASCII to numeric conversion and subtract x41 or 65 if you are a normal human and think numerically in decimal.
The correct answers array is not going to cut it. It’s actually in human alpha. Completely useless.
What we need is correct answers in a format that I can relate to. You might fall in place behind me and agree by the end of this. Maybe not, got to get there first.
Oops, this is a binary not decimal thing. 0,1,2,3 is not going to do.
Take the 0,1,2,3 and raise them to the power of 2.
0001
0010
0100
1000
That looks even better.
We could do this bitwise but that would require an if else. We do do not like if else. Intel microprocessors do not like if else either. Actually the reason I do not like if else is because processors do not like if else. In this business it’s a good idea to be on the same side as the brain.
Let’s separate the binary bits and create an array. Got to keep the micro happy.
0,0,0,1
0,0,1,0
0,1,0,0
1,0,0,0
No something is missing. What is it? Do you know? Nothing. This is perfect.
What do we have so far? Let’s think about that.
User answers in the format of (and 50% less memory) 0,1,2,3 rather than A,B,C,D. Are we going to be graded on this?
It is important to keep keep memory structures small and aligned on address boundaries.
Intel 64 and IA-32 Architectures Optimization Reference Manual : 3.6.6 Data Layout Optimizations User/Source Coding Rule 6.(H impact, M generality) Pad data structures defined in the source code so that
every data element is aligned to a natural operand size address
boundary. If the operands are packed in a SIMD instruction, align to
the packed element size (64-bit or 128-bit). Align data by providing
padding inside structures and arrays. Programmers can reorganize
structures and arrays to minimize the amount of memory wasted by
padding.3.6.4 Alignment Misaligned data access can incur significant performance penalties. This is particularly true for cache line
splits. The size of a cache line is 64 bytes in the Pentium 4 and
other recent Intel processors, including processors based on Intel
Core microarchitecture. An access to data unaligned on 64-byte
boundary leads to two memory accesses and requires several µops to be
executed (instead of one). Accesses that span 64-byte boundaries are
likely to incur a large performance penalty, the cost of each stall
generally are greater on machines with longer pipelines.
OK, this is too much. We got correct answers and incorrect answers. Like I said, too much. One or the other, not both. which? We’ll wing it for now.
We are told to create a new array for answers. Do we not already have one passed to us? Do we really want to waste electrons doing that? I don’t but that’s up to you.
for (int i = 0; i < Answers.length; i++){
userAnswers[i] = Answers[i];
}
Never having written a line of java before this, I am not an authority on the syntax to say the least. But there should be an easier way to copy an array. Maybe like:
System.arraycopy( Answers, 0, userAnswers, 0, Answers.length);
How about kill two birds with one stone? Maybe more. More is better?
ASCII to numericord()
ord((userAnswers[i])
Mask off the lower case bit with 0xdf (0b11011111) to
convert lower to upper case.
ord((userAnswers[i]) & 0xdf))
And convert from ASCII ord() value to 0-3
subtract 0x41 to convert ‘A’ to zero, B to one, and etc.:
ord((userAnswers[i]) & 0xdf)) - 0x41)
Putting it all together:
for (int i = 0; i < userAnswers.length; i++){
correctCount += correctAnswers[i][(ord((userAnswers[i]) & 0xdf)) - 0x41)] ;
}
Oh! Not much left to do! But the above is nearly the whole 9 yards.
Well if you haven’t figured it out by now, here it the correctAnswer array converted to multi-dimensional array:
Where the correct alpha answer equates to the following:
A = [0] 1,0,0,0
B = [1] 0,1,0,0
C = [2] 0,0,1,0
D = [3] 0,0,0,1
correctAnswers = new int[][]{{0,1,0,0},{0,0,0,1},{1,0,0,0},{1,0,0,0},{0,0,1,0},{1,0,0,0},{0,1,0,0},{1,0,0,0},{0,0,1,0},{0,0,0,1},{0,1,0,0},{0,0,1,0},{0,0,0,1},{1,0,0,0},{0,0,0,1},{0,0,1,0},{0,0,1,0},{0,1,0,0},{0,0,0,1},{1,0,0,0}};<br>
One line of code to get the number of correct answers:
for (int i = 0; i < Answers.length; i++){ correctCount += answers[i][(ord(userAnswers[i] & 0xdf) - 0x41)];}
Because the value of correctAnswers[i][(ord(userAnswers[i] & 0xdf) - 0x41)]; is either 1 for correct and zero for incorrect:
You can store the answers in a 2 dimensional array where correct answers are array[1] and incorrect answers are array[0]:
for (int i = 0; i < Answers.length; i++){
answersScored[(correctAnswers[i][(ord(userAnswers[i] & 0xdf) - 0x41)]][i]= 1;
}
The above will give an array of both correct and incorrect answers.
Correct Answers answersCorrect:
System.arraycopy( answersScored[1], 0, answersCorrect, 0, answersScored[1].length);
And Incorrect answersIncorrect:
System.arraycopy( answersScored[0], 0, answersIncorrect, 0, answersScored[0].length);
What’s the Point?
The use of arrays rather than if else is an unconventional programing methodology today. It is not widely known and because it is unconventional it is often Misunderstood.
The reason the array method is preferred over using an if else, is that if else structures should be avoided whenever possible. When looking at how the code executes in the microprocessor’s execution engine. Intel has gone to extreme lengths to deal with branching and branch prediction.
Intel’s recommendation in their 64 and IA-32 Architectures Optimization Reference Manual they make it very clear to remove branches whenever possible.
3.5.3 User/Source Coding Rule 4. (M impact, ML generality) Avoid the use of conditional branches inside loops
Intel’s latest micro-architectures the have implemented Out of Sequence Instruction Execution where up to 8 instructions can be executed in one clock cycle. This technique does not work well with in conjunction with branch prediction.
While traditional programming uses an abundance of if else structures, it is not recommended in high performance designs.
In this DMV app it is not crucial, but when there are thousands of lines of code in a loop structure it is of paramount importance to use efficient coding techniques which have as few if else structures as possible.