hardyboy16jm
hardyboy16jm

Reputation: 91

Android/Java: Enum methods crashing app

My enum declaration:

public enum Note { A, A_SHARP, B, C, C_SHARP, D, D_SHARP,
    E, F, F_SHARP, G, G_SHARP;

    public String toString(Note note) {
        if (note == Note.A)
            return "A";
        else if (note == Note.A_SHARP)
            return "A#";
        else if (note == Note.B)
            return "B";
        else if (note == Note.C)
            return "C";
        else if (note == Note.C_SHARP)
            return "C#";
        else if (note == Note.D)
            return "D";
        else if (note == Note.D_SHARP)
            return "D#";
        else if (note == Note.E)
            return "E";
        else if (note == Note.F)
            return "F";
        else if (note == Note.F_SHARP)
            return "F#";
        else if (note == Note.G)
            return "G";
        else if (note == Note.G_SHARP)
            return "G#";
        else
            return "";
    }

    public Note getNext() {
        int index = ordinal();
        index++;
        if (index > values().length)
            return values()[0];
        else
            return values()[index];
    }
}

Whenever I call either of these two methods, my app crashes. Here is where I call them:

public void ChangeSound(View v) {
    note = note.getNext();
    tvSounds.setText(note.toString(note));
}

ChangeSound() is an onClick method for a button. If I remove both lines in ChangeSound(), the code works as it should, but if either of the two lines are in there, the app crashes on the button click. Any ideas why? Thanks in advance!!

EDIT** note is a variable of type Note

Thank you everyone! It was returning null (look at Jason C's answers (my comment)). All of this was helpful for me!

Upvotes: 2

Views: 492

Answers (4)

Jason C
Jason C

Reputation: 40376

It's unclear what you mean by "crashes", and you also do not show enough context (what is 'note'?) but the most likely cause based on the fact that you stated either of those two lines crash seems to be that 'note' is null. If 'note' is null then ChangeSound will throw a NullPointerException. You need to make sure that if ChangeSound is assuming 'note' is not null, that that is actually the case.

Also you should make toString(Note) a static method, and define a non-static toString() override. This will give Note.toString(Note) the ability to handle nulls correctly:

public static String toString (Note n) {
    return n == null ? "" : n.toString();
}

Edit: As noted in other answers, you should use >= instead of > (even == would be sufficient), that is also a potential problem.

Upvotes: 0

assylias
assylias

Reputation: 328775

The issue has already been spotted by the other answers. Note however that you could simplify your code in two ways:

  • by associating the String representation of the notes with the enum constants directly
  • by using a modulus instead of your if/else in getNext

It could look like:

public enum Note { A("A"), A_SHARP("A#"), B("B"), C("C"), C_SHARP("C#"), D("D"),
      D_SHARP("D#"), E("E"), F("F"), F_SHARP("F#"), G("G"), G_SHARP("G#");

    private final String noteName;
    Note(String noteName) {
        this.noteName = noteName;
    }

    @Override
    public String toString() {
        return noteName;
    }

    public Note getNext() {
        int nextIndex = (ordinal() + 1) % values().length;
        return values()[nextIndex];
    }
}

And in your main code:

note = note.getNext();
tvSounds.setText(note.toString());

Upvotes: 0

splungebob
splungebob

Reputation: 5425

Change this:

if (index > values().length)

to this:

if (index >= values().length)

Upvotes: 2

Tala
Tala

Reputation: 8928

You should use >= since probably you're getting some OutOfBoundsException

if (index >= values().length)
        return values()[0];
    else
        return values()[index];
}

Also instead of switch you could sth like that:

public enum Note { A("A"), A_SHARP("A#"), B("B");

private String s;
public Note (String s) {
    this.s = s;
}
public String toString() {
    return s;
}

Upvotes: 4

Related Questions