Student Grades Calculator

I am seeking a review of a previous project I completed in my first term at university. The submission was graded 18/20 however the feedback was minimal and gave no suggestions regarding further improvements.

The specification prohibited the use of any non-primitive data types.

Concise problem specification:


Students take 6 modules in Stage 1. Each module has two components -
Exam and Coursework and each module can have a different mark for
each. The returned mark for a module is calculated as follows:


computed module mark = ((coursework mark * coursework weighting) +
(examination mark * (100 - coursework weighting))) / 100


If the exam mark and coursework mark are greater than or equal to 35
then the returned mark is the computed module mark. However, if either
(or both) the exam or coursework mark is less than 35 then the
returned mark is the minimum(35, computed module mark).


All marks are rounded to the nearest whole number.


The student's performance on a given module can then be recorded as
one of:

• Pass, if the module mark is at least 40

• Compensatable Fail, if the module mark is less than 40 but at least 35

• Fail, otherwise

Once all the module marks have been determined, a Stage average may be
computed. This is found by averaging the returned module marks. The
Stage result can then be computed as:

• Pass, if all modules are recorded as a Pass

• Pass By Compensation, if the Stage average is at least 40, no module is recorded as a Fail, and there are one or two modules

totalling at most 40 credits recorded as a Compensatable Fail.

• Fail, otherwise

The submission is required to produce a clear formatted output showing the marks of each module in addition to the overall stage
result. A graph should also be generated to reflect these results.

Sample output log:

```
Module 1 - Coursework Weighting: 50 - Exam Mark: 80 - Coursework Mark: 75
Module 2 - Course

Industry standard rarely would restrict you to sending and receiving only primitive data. There are a few reasons for it, but mostly its so that the code can be read easier, and meaning can be associated with numbers. In your example instead of passing in a multi-dimensional array (which gives little in the way of meaning to numbers) you would instead pass in a collection of some class. It equates to the same in the end, but sure is easier to read. I know you said that it was part of the requirements, but I couldn't help but speak up to this. On to your code: I am not a big fan of having a class with only public static methods. There are reasons for it, but in general it's not good practice. I've been seeing more and more people starting to prefer constructor parameters for a class. The idea is that you give the class everything it requires to start work, then call a method to perform said calculation. How you get the calculation depends you could either return the value, or have the method set a field in your class that has a public getter. Here is basically what I'm saying in code

Option one

public class MarkCalculator {
    private final int[][] studentData;

    public MarkCalculator(int[][] studentData) {
        this.studentData = studentData;
    }

    public int[] computeMarks() {
        //...
    }
}

Option two

public class MarkCalculator {
    private final int[][] studentData;

    private int[] marks;

    public MarkCalculator(int[][] studentData) {
        this.studentData = studentData;
    }

    public void computerMarks(){
        //...
    }
    public int[] getMarks(){
        return marks;
    }
}

For this scenario I think I would prefer to return void, and have a public getter for marks. The reason I would prefer that is it would appear (based on the commented out code, and the graph that you show) that you want to do different things based on this specific data. On the classes that you want to use this data you would pass in an instance of MarkCalculator. This tells people who are going to use the data that it was made specifically for this the data that was computed for MarkCalculator. Now this isn't always the case and you'll want to weight the pro's and con's before you commit to one style or another.

I caught in the code that you have "debug" code in. I have two things to say about this: 1) Create a class that shows said data. 2) other programmers might not like using the console to inspect data nor do they want to see the clutter of commented out debug code. Using an automated test framework can fix this. Java has a few different testing frameworks, but I'm not going to promote any particular one since they are similar in syntax and in reality they accomplish the same thing. These tests are a way to address both points. The first point is covered because it is a class that, typically, has "test" in its name. Other programmers will want to look for tests to know how to use the class and see what type and format the data needs to be in. With the tests isolated from production code you don't clutter up the production code with "debug" code. With that in mind here is a test I started to make and came across a potential refactoring point/bug fix

@Test
public void testComputeMarks() throws Exception {
    int[][] data =
        {
            {50,80,75},
            {50,35,60},
            {50,70,25},
            {50,70,40},
            {50,80,50},
            {50,15,70}
        };
    MarkCalculator markCalculator = new MarkCalculator(data);
    int[] marks = markCalculator.computeMarks();

    assertEquals(marks[0], 77);
    assertEquals(marks[1], 47);
    assertEquals(marks[2], 35);
    assertEquals(marks[3], 55);
    assertEquals(marks[4], 65);
    assertEquals(marks[5], 35);
}

I noticed that when writing this you require that you'll ALWAYS have 6 modules. This would mean that MarkCalculator could be enhanced if you were to make it more dynamic. Tests are a nice way of informing you (very quickly) if the changes made to a class are breaking anything (I say very quickly because most tests run in less than 1 second and they can be run repetitively). When it fails my IDE shows a big red X and red progress bar saying something failed and a description of why it failed. Likewise if it passes I get a nice green check mark and progress bar. Now lets dig into your code. It takes less than a a second to run this test so I'll run it frequently.

I see that computedModuleMark is declared outside of where it is used. Its an integer and so does not need any resources cleaned up, nor is it used outside of that method. Therefor its declaration and value should be on the same line. After doing that I check my test and it still passes. Also you have a hard coded the size of marks, yet it should be dynamic to match the number of modules. I changed it to int[] moduleMarks = new int[studentData.length]; and run my tests and it still works. Moving down we se

StackExchange Code Review Q#114684, answer score: 6