Playing sound in JavaFX when button is pressed, window pops up

Question

So I made a class for playing a sound in JavaFX when a button is pressed, a window pops up... I don't know about the quality of the code though. I don't know if I used the best way to play the sound. It's meant to be used to play small mp3 or wav files (not longer than 5 seconds) I made it so you can adjust the volume of it (i.e. an option menu where you can adjust your notification sound menu)

Here's the code:

public class Sound {

String file;
Media sound;
MediaPlayer player;

public Sound(String file) {
    this.file = file;
    this.sound = new Media(new File(file).toURI().toString());
    this.player = new MediaPlayer(sound);
}

public void play(){
    player.play();
}

public void stop(){
    player.stop();
}

public void setVolume(double value){
    player.setVolume(value);
}

public double getVolume(){
    return player.getVolume();
}
}


//A little GUI with a button that plays a sound and a volume slider for that sound

public class MusicPane extends Pane {


String path;
Sound sound;
public MusicPane(String path) {

    this.path = path;
    this.sound  = new Sound(path);

    //if this button is pressed, it plays the sound
    Button button = new Button("play");
    button.setOnAction(new EventHandler<ActionEvent>() {
        @Override
        public void handle(ActionEvent e) {
            //first stop a playing sound, then start the sound
            sound.stop();
            sound.play();
        }
    });

    //Slider to adjust the volume
    Slider slider = new Slider();
    slider.setMin(0);
    slider.setMax(1.0);
    slider.setValue(sound.getVolume());

    slider.valueProperty().addListener(new InvalidationListener() {
        @Override
        public void invalidated(Observable observable) {
            sound.setVolume(slider.getValue());
        }
    });
    this.getChildren().addAll(button, slider);
}
}

Answer 1

It's generally more standard to hide as many fields of your class as possible. I'd recommend making file, sound and player in Sound as well as path and sound in MusicPane private.

Additionally you're not using file in Sound, so you can remove it. Furthermore your class' bodies are not indented by one level (contrary to usual conventions).

Aside from that there's not much to say about Sound the only thing that may be improved is using Paths.get instead of new File when you create your sound.

The MusicPane has a bit more to talk about. For one I don't quite understand why you're not using an fxml to specify the components. Additionally conventional wisdom suggests to use composition over inheritance. Instead of subclassing Pane into a MusicPane, you should use the Pane as a Member of the "GUI" you expose to your program.

This allows you to better share behaviour between components you create by subclassing Components instead of Pane.

The comments in the code aren't really useful... // if this button is pressed, it plays the sound can be completely removed, especially when you rename the button to play.
//first stop a playing sound, then start the sound can be completely removed. It adds no information whatsoever to the code.
//Slider to adjust the volume becomes completely superfluous as soon as you rename slider to volumeSlider.