1

I'm creating simple game in Java which plays music in background.

Music class contains those static methods:

  • playMusic();
  • stopMusic();
  • changeVol(int vol);

At the very begin I'm crating new object of 'Music' class and I'm calling static method 'playMusic':

    new Music();    
    Music.playMusic();

The reason why those methods are static is that I want to start playing music when application starts but You can chose if music is played or not in settings. Static methods provide the easiest way of doing it since 'Settings' class does`t have 'main' method.

The Music class code: 

public class Music {
/// ... variables   

public Music(){     
    try{
        /// ... Preparing clip to play
        } catch (Exception e){
            /// ... Catching exception
        }
}

public static void playMusic(){
     /// ... starting music       
}

public static void stopMusic(){
      /// stoping music
}

public static void changeVol(int vol){
    /// changing volume 
}}

It works perfectly fine but I wondering if it's correct with OOP and Java standards?

Full Music class code: http://pastebin.com/d9XYX1gG

Improve this question
6
  • 9
    Why are you creating an object if you aren't using it? That doesn't make sense. Commented Mar 19, 2015 at 15:11
  • 2
    It looks very smelly even without seeing what's inside your methods. I think it will be much cleaner if you re-write your logic to use instance methods. What is the problem with this? Simply create an instance of Music as your application starts up. Commented Mar 19, 2015 at 15:12
  • 3
    A singleton will be more appropriate for your music player as you can save the music player's state if needed instead of having ugly static class members (and still access it globally) Commented Mar 19, 2015 at 15:12
  • Some say that using too many static methods is violation of OOP... Commented Mar 19, 2015 at 15:13
  • No need to create an object. All the methods are static and you can call them directly through their class name as you are doing here. Music.playMusic(); Commented Mar 19, 2015 at 15:17

3 Answers 3

Reset to default
3

What you probably want to do is something like this:

Music music = new Music();
music.playMusic();

No need for static methods here.

If you want only one instance which can be accessed from several places, try using a Singleton:

public class Music {

    private static instance Music instance;

    private Music() {
        // initialize your stuff here
    }

    // have your instance methods here

    /**
      * This is the method to access the Music singleton
      */
    public static Music getInstance() {
        if (instance == null) {
            instance = new Music();
        }
        return instance;
    }
}
Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

Or with static methods: Music.playMusic();
@Radek That's exactly what the OP is doing right now. You could have a static initializer of course, thereby getting rid of the constructor.
@blalasaadri great but how can I access this object in two different classes then?
@PoQ The best solution would probably be to have it as a singleton. So, you have a private constructor and one static method (often called something like getInstance()) which returns the one static instance (which it creates the first time it's called). I've added an example for that in my answer.
2

No, you do not need to create an object to call the static methods. So...

Music.playMusic();

...will work fine without creating a new instance first.

I also do not believe that static is really required here. You can do your initialization of your program, read the settings, etc. and then create a Music object and start the music - or not, depending on the settings.

Improve this answer

2 Comments

The instance is probably created because the constructor does some initialization. Not ideal of course, the initialization could be done statically as well if no instance is needed.
Good guess, but honestly, calling that "not ideal" is a very friendly way of putting it ;-)
2

No, this is not the correct way to use OOP. By using the static keyword you are making the class have methods that do not require an object and would not have any data encapsulation (on an instance level).

If you do want to use static methods you can just write Music.playMusic(), however your constructor would not be called. When you use static methods you do not have access to instance variables, so whatever you are doing to prepare in the constructor may not be accessible to your static methods since you cannot use the this keyword.

Ideally you should remove static from each of your methods, pass anything you want to the constructor (if necessary) and then call as such:

//assume file is a music file
Music music = new Music(file);
music.playMusic();

This will use the passed music file assume that's what you were preparing in the constructor. If you only have one of these throughout your program you can utilize the Singleton Pattern.

If you still want to go the static route and you need to prepare something one-time before any static methods are called you should use what's known as a class initializer. To use this you would remove your constructor because you don't need to initialize objects, and instead replace it with a method that looks like this within your class:

static{
    try{
        /// ... Preparing clip to play
     } catch (Exception e){
        /// ... Catching exception
     }
 }

Keep in mind that a class initializer is guaranteed to run before you call any methods in the class (static or object level), but is run at the time the class is loaded, not necessarily on the first call of a static method.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.