Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have the following code to write:

Modify the Student class so that instead of 3 tests you will now have an array of 100 tests. However, not all of the scores may be set. You need to add an additional field to keep track of the number of tests currently being stored. You will need to modify setTest and getTest as well. Also, add a method titled addTest that will add a new test score in the appropriate location. This is the code I have to write/modify and this is what I've done with it.

package net.apcs.classes;

import java.util.Arrays;

public class Student {

    private String FirstName;
    private String LastName;
    private double[] Tests = new double[100];
    private int NumberOfTests;

    public Student(String firstName, String lastName, double[] tests, int numberOfTests) {
        FirstName = firstName;
        LastName = lastName;
        tests = new double[100];
        NumberOfTests = numberOfTests;
    }

    public Student() {
        FirstName = "";
        LastName = "";
        Tests = new double[100];
        NumberOfTests = 0;
    }

    public String getFirstName() {
        return FirstName;
    }

    public String getLastName() { return LastName; }

    public double getTest(int testNum) {
        return Tests[testNum];
    }

    public void setFirstName(String firstName) {
        FirstName = firstName;
    }

    public void setLastName(String lastName) {
        LastName = lastName;
    }

    public void setTest(int testNum, double testScore, double[] tests) {
        tests[testNum] = testScore;
    }

    public double getAverage(double[] tests, int numberOfTests) {
        int i = 0;
        double sum = 0;
        while (i < numberOfTests) {
            sum = sum + tests[i];
            i++;
        }
        double average = sum / numberOfTests;
        return average;
    }

    public double getHighScore(double[] tests, int numberOfTests) {
        double max = 0;

        for (int i = 0; i < numberOfTests; i++) {
            if (tests[i] > max) {
                max = tests[i];
            }
        }

        return max;
    }

    @Override
    public String toString() {
        return "Student{" +
                "FirstName='" + FirstName + '\'' +
                ", LastName='" + LastName + '\'' +
                ", " + Arrays.toString(Tests);
    }
}

Could somebody look over this to see if I implemented the arrays properly? I'm not sure if I used them properly and if the code will do what it's supposed to. (Before anyone asks, I don't know how to create an object with an array and test the program to see if it works so if someone could tell me how to do that as well, it would be much appreciated).

share|improve this question
1  
You'll have to determine if it works properly. We cannot review it in case it's broken. We also cannot assist with adding new implementation (the last statement). – Jamal Feb 13 '16 at 0:37

Java constructors

You say

    private double[] Tests = new double[100];

but then in the constructor, you say

        Tests = new double[100];

One of those is unnecessary. You are overwriting the first with the second. So you could get rid of either one and the code behavior would stay the same.

And in the other other constructor, you say

    public Student(String firstName, String lastName, double[] tests, int numberOfTests) {
        FirstName = firstName;
        LastName = lastName;
        tests = new double[100];
        NumberOfTests = numberOfTests;
    }

In Java, the standard is to write class names StudlyCase and variable names camelCase. Here, you write variable names StudlyCase. It would be more common to write this

    public Student(String firstName, String lastName, int numberOfTests) {
        this.firstName = firstName;
        this.lastName = lastName;
        tests = new double[numberOfTests];
    }

Notice how I do not pass in an array. Your original code wrote over but never used that array. This code sets the field tests (the camelCase form of Tests) in the object based on the number of tests. It does not have to save the number of tests, because the array stores that information in Java. You could also say

    public Student(String firstName, String lastName, double[] tests) {
        this.firstName = firstName;
        this.lastName = lastName;
        this.tests = tests;
    }

This would normally be used to set the test values at the time of construction.

    public Student() {
        FirstName = "";
        LastName = "";
        Tests = new double[100];
        NumberOfTests = 0;
    }

This seems pointless. You have no way of changing NumberOfTests later. So why have it?

Setters should change the object

    public void setTest(int testNum, double testScore, double[] tests) {
        tests[testNum] = testScore;
    }

Why would setTest set a value in an array that you pass to it?

Lists would make more sense than an array

This particular use case seems better suited to a List than an array. With a list, you could say

    public void addTest(double testScore) {
        tests.add(testScore);
    }

and it would automatically resize and put the test in the right spot. That way you wouldn't have to know how many tests there were before adding.

But of course, you may be required to use an array.

Iterate with for rather than while

You say

    int i = 0;
    double sum = 0;
    while (i < numberOfTests) {
        sum = sum + tests[i];
        i++;
    }

But you could write this more idiomatically as

    double sum = 0;
    for (int i = 0; i < tests.length; i++) {
        sum += tests[i];
    }

and even easier as

    double sum = 0;
    for (double test : tests) {
        sum += test;
    }

Now you don't have to care about the loop variable. Java will handle that for you.

Note that I also shortened sum = sum + test to just sum += test. Both statements do the same thing, but the latter is shorter.

Class vs. object methods

In a method like one of

    public void setTest(int testNum, double testScore, double[] tests) {
    public double getAverage(double[] tests, int numberOfTests) {
    public double getHighScore(double[] tests, int numberOfTests) {

You would either make these static class methods or remove the parameters that already exist in the class. In this case, the latter seems better

    public void setTest(int testNum, double testScore) {
    public double getAverage() {
    public double getHighScore() {

Now these methods would be based on just what is in the class. The caller doesn't have to maintain arrays. That's why you have classes and objects, so that callers can abstract away from the things defined in the class.

Using an array for storage already puts extra strain on the caller. The caller needs to know how many tests there are before creating the Student object. Or you need to reinvent List functionality using an array.

main

@Jamal said that we wouldn't write test code for you, but I'm going to give you a bit of a push:

    public static void main(String[] args) {
        double[] scores = {100.0, 50.0, 90.0};
        Student test = new Student("Test", "Student", scores);
        System.out.println("Average:  " + test.getAverage());
    }

This initializes a student with three test scores and prints out the average score.

It would be better to test with unit tests, but this at least produces some output. You'll have to write additional tests on your own. The easy way to use this is to add it as a method for Student. The more robust way would be to add a new class called something like Main and put it there.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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