Model of an mp3 player with state and prototype design patterns

After reading about design patterns in general, I decided to try and incorporate them in an actual program. I chose a State machine for my first exercise, or, more concretely, an MP3 player. The player itself does not play any audio files, it just models the behavior of one.

The MP3Player behavior is as follows:

• If the play button is pressed, play current song, if no song is currently playing
• If the pause button is pressed:
  • If song is already paused, reset current to first song in the list
  • If songs are already reset, do nothing
  • Else, pause current song
• If the forward button is pressed, pause song and move current to next in the list
• If the rewind button is pressed, pause song and move current to the previous in the list

I’d like feedback on:

• Correct implementation of State and Prototype patterns
• Whether those two patterns are a right approach in this particular case

I have a State interface that includes all methods, and an AbstractState with the initial implementation of those methods.

---

State.java

interface State {
    public void play(MP3Player player);
    public void pause(MP3Player player);
    public void forward(MP3Player player);
    public void rewind(MP3Player player);
}

StateCache.java

class StateCache {
    private static HashMap<String, AbstractState> states = new HashMap<>();

    public static void loadCache() {        
        PlayState playState = new PlayState();
        states.put("PlayState", playState);

        PausedState pausedState = new PausedState();
        states.put("PausedState", pausedState);

        ResetState resetState = new ResetState();
        states.put("ResetState", resetState);
    }

    public static AbstractState getState(String shapeID) {
        AbstractState cachedState = states.get(shapeID);
        return cachedState.clone();
    }
}

AbstractState.java

abstract class AbstractState implements State, Cloneable {
    static {
        StateCache.loadCache();
    }

    @Override
    public abstract void play(MP3Player player);

    @Override
    public abstract void pause(MP3Player player);

    @Override
    public void forward(MP3Player player) {
        System.out.println("Forward...");
        player.increaseCurrent();

    }

    @Override
    public void rewind(MP3Player player) {
        System.out.println("Reward");
        player.decreaseCurrent();
    }

    @Override
    public AbstractState clone() {
        AbstractState result = null;
        try {
            result = (AbstractState) super.clone();
        }
        catch (CloneNotSupportedException e) {
            e.printStackTrace();
        }

        return result;
    }

}

PlayState.java

class PlayState extends AbstractState {

    @Override
    public void play(MP3Player player) {
        System.out.println("Song is already playing");
    }

    @Override
    public void pause(MP3Player player) {
        System.out.println(String.format("Song %d is paused", player.current));
        player.setState(StateCache.getState("PausedState"));
    }

    @Override
    public void forward(MP3Player player) {
        super.forward(player);
        player.setState(StateCache.getState("PausedState"));
    }

    @Override
    public void rewind(MP3Player player) {
        super.rewind(player);
        player.setState(StateCache.getState("PausedState"));
    }

}

PausedState.java

class PausedState extends AbstractState {

    @Override
    public void play(MP3Player player) {
        System.out.println(String.format("Song %d is playing", player.current));
        player.setState(new PlayState());
    }

    @Override
    public void pause(MP3Player player) {
        System.out.println("Songs are stopped");
        player.current = 0;
        player.setState(StateCache.getState("ResetState"));
    }

}

ResetState.java

class ResetState extends PausedState {

    @Override
    public void pause(MP3Player player) {
        System.out.println("Songs are already stopped");
    }
}

Song.java

public class Song {
    private String title;
    private String artist;

    public Song(String title, String artist) {
        this.title = title;
        this.artist = artist;
    }

    public String title() {
        return title;
    }

    public String artist() {
        return artist;
    }

    @Override
    public String toString() {
        StringBuilder result = new StringBuilder();
        result.append(String.format("Song{title=%s, artist=%s}", title, artist));
        return result.toString();
    }
}

MP3Player.java

public final class MP3Player {
    private List<Song> songs;
    private State currentState;
    int current;


    protected void setState(State newState) {
        currentState = newState;
    }

    protected void increaseCurrent() {
        current++;
        if (current >= songs.size())
            current -= songs.size();
    }

    protected void decreaseCurrent() {
        current--;
        if (current < 0)
            current += songs.size();
    }

    public MP3Player(List<Song> songs) {
        this.songs = new ArrayList<>(songs);
        current = 0;
        currentState = new PausedState();
    }

    public void printCurrentSong() {
        System.out.println(songs.get(current));
    }

    public void pressPlay() {
        currentState.play(this);
    }

    public void pressPause() {
        currentState.pause(this);
    }

    public void pressFWD() {
        currentState.forward(this);
    }

    public void pressREW() {
        currentState.rewind(this);
    }

    @Override
    public String toString() {
        StringBuilder result = new StringBuilder();
        result.append(String.format("MP3Player{currentSong = %d, songList = %s}", current, songs.toString()));
        return result.toString();
    }
}

My annotations:

1. Remove the interface State. It’s not necessary. You already have an abstraction with “AbstractState”
2. ResetState should not derive from PausedState they have nothing to do with each other. Inheritance is not for reducing code redundancy. It’s modelling semantic. Maybe you mean that AFTER you made a Reset (go into ResetState) you are going to the PauseState.
3. The same with the methods in AbstractState that were involved in the state machine. pull them down to the concrete states. Code is not necessarily redundant if it looks the same. In the AbstractState the methods involved in state-change should at most implemented empty.
4. Remove all the super-calls
5. Pass the Mp3Player through the constructor to the states and not through the methods
6. As you are using the flyweight pattern right now for your states keep them immutable in the future.
7. As you are using the flyweight pattern no prototype pattern is neccessary. Why would you create something out of a prototype if the result will always remain like the prototype?
8. remove static initializer static {…} and introduce lazy initialization in your StateCache. As you have no access to the Mp3Player in the StateCache do the caching in the Mp3Player. The holder object and the state objects are high cohesional so do not pull them apart. You should not let other objects than the holder object or other states create them

Nevertheless: In this use case (an Mp3 Player) I would not cache any state object. They can be instantiated on demand. If you have a user of your Mp3Player who is clicking the Buttons million times a second we can talk about another solution.

But overall a good first implementation.