java - Adding a variable to an array

Question

I'm having trouble writing this instance method.. I'm trying to write the method so that the method will check if there is space for another competitor. And if there is, another competitor will be added in the next available slot.

    public boolean addCompetitor(Competitor competitor) {
    // TODO
    for(int i=0;i<competitors.length; i++){
        if(numberOfCompetitors < MAX_COMPETITORS){
            numberOfCompetitors++;
            return true;
        }
    }

    return false;
}

I've done a loop to see if I can just add the variable to the array if the conditions met true.

This is the full error output.

java.lang.NullPointerException
at Race.finishRace(Race.java:71)
at TestA2Classes.start(TestA2Classes.java:46)
at TestA2.main(TestA2.java:12)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
at java.lang.reflect.Method.invoke(Unknown Source)
at edu.rice.cs.drjava.model.compiler.JavacCompiler.runCommand(JavacCompiler.java:272)

Full code:

public class Race {
public static final String[] RACE_DESCRIPTIONS = {"Sprint", "Distance", "Eliminator", "Keirin"};
public static final int SPRINT = 0;
public static final int DISTANCE = 1;
public static final int ELIMINATOR = 2;
public static final int KEIRIN = 3;
public static final int MAX_COMPETITORS = 8;

private int number;
private int typeIndex;
private MyDate date;
private boolean hasFinished;
private Competitor[] competitors;
private int numberOfCompetitors;

public Race(int number, int typeIndex, MyDate date) { 
    // TODO
    this.number = number;
    this.typeIndex = typeIndex;
    this.date = date;
    this.hasFinished = false;
    this.numberOfCompetitors = 0;
    this.competitors = new Competitor[MAX_COMPETITORS];



}


public int getNumber() {
    // TODO
    return number;
}

public boolean getHasFinished() {
    // TODO
    return hasFinished;
}

public int getTypeIndex() {
    // TODO
    return typeIndex;
}

public MyDate getDate() {
    // TODO
    return date;
}

public Competitor getCompetitor(int number) {
    // TODO
    for(int i=0; i<competitors.length; i++){
        if(competitors[i].getNumber() == number){
            return competitors[i];
        }
    }
    return null;
}


public void finishRace(int first, int second, int third) { 
    // TODO
    this.hasFinished = true;
    for(int i=0; i<competitors.length; i++){
        if(competitors[i].getNumber() == first){
            competitors[i].setPosition(1);
        } else if(competitors[i].getNumber() == second){
            competitors[i].setPosition(2);
        } else if(competitors[i].getNumber() == third){
            competitors[i].setPosition(3);
        } else{
            competitors[i].setPosition(0);
        }
    }
}

public boolean addCompetitor(Competitor competitor) {
    // TODO
    if(numberOfCompetitors < MAX_COMPETITORS){
        competitors[numberOfCompetitors] = competitor;
        numberOfCompetitors++;
        return true;
    }

    return false;
}

public String toString() {
    // TODO
    String details = number + ", " + RACE_DESCRIPTIONS[typeIndex] + " [" + date + "]";
    if(!hasFinished){
        details += ": Race not finished";
    } else if(hasFinished){
        details += "
     1st: " + competitors[0].getName();
        details += "
     2nd: " + competitors[1].getName();
        details += "
     3rd: " + competitors[2].getName();
    } else{
        details += "n/a";
    } 
    return details;
}

}

Answer 1

As it stands, if your condition is false in your for loop, then it will be false at ALL iterations, and you're just wasting computation time. The if-statement on it's own will suffice.

public boolean addCompetitor(Competitor competitor) {
    if(numberOfCompetitors < MAX_COMPETITORS)
    {
        competitors[numberOfCompetitors++] = competitor;
        return true;
    }
    return false;
}

You know how many competitors you have entered so far. You need not a loop, just check if you've added too many directly.

A loop would be necessary if you didn't already save how many competitors you've added like this(this requires that no legitimate entries can be null, or it will be overwritten):

public boolean addCompetitor(Competitor competitor)
{
    for(int i = 0 ; i < competitors.length ; i++)
    {
        if(competitors[i]==null)
        {
            competitors[i] = competitor;
            return true;
        }
    }
    return false;
}

This approach will take longer the more competitors you've added (O(n)), while the other approach always takes the same amount of time (O(1))

Addressing the error on the new edit

for(int i=0; i<competitors.length; i++){

You're looping through your whole competitors array, which may not have competitors yet.

Try changing it to

for(int i=0; i<numberOfCompetitors; i++){