Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a huge list of song objects in my program and I need those objects in almost all activities. Well, at least a part of it up to everything.

So I created a class which looks pretty much like this :

class DataStore {

   private static ArrayList<Song> songList;
   private static ArrayList<Album> albumList;

   private DataStore()
   {
   }

   //getters and setters for the private variables above
   public void getSongList() { ... }
   ...


}

So this DataStore has a lot of private variables and getters / setters to access them. I call these functions like this :

ArrayList<Song> songList = DataStore.getSongList();

Now I am wondering, is that a good approach? Should I do anything different to create global variables I can use in every activity?

Also, I saw a few questions about Singletons. Is this a Singleton class?

share|improve this question

2 Answers 2

up vote 4 down vote accepted

I have a huge list of song objects in my program and I need those objects in almost all activities.

Your description is an excellent fit for Content Providers.

http://developer.android.com/guide/topics/providers/content-providers.html

Implementing a content provider for your songs might seem like a lot of work at first, but it will be worth the investment. It's the clean and recommended approach, and you'll benefit from it greatly. As an added bonus, other applications will be able to use your song "database" too. Go for it.

About the singleton pattern, read this:

http://en.wikipedia.org/wiki/Singleton_pattern

And avoid using singletons as much as possible.

share|improve this answer

Yes, this is a Singleton class. Or actually, technically this is not even a singleton as it doesn't appear to be instantiated at all.

What makes it similar to a singleton class though is that there is only, and can be only one variable of these:

private static ArrayList<Song> songList;
private static ArrayList<Album> albumList;

Overall, it is a bad idea to have static variables like this.

Some few additional comments:

  • Make your variables final
  • Declare as List and instantiate as ArrayList

So this would be better:

private static final List<Album> albumList = new ArrayList<Album>();

However, ask yourself these two questions: Does the order of the albums matter? Should it be possible to have two equal objects in the list? If the answer to both these questions is No, then it would be better of as Set<Album> albums = new HashSet<Album>();

Additionally, you can see a previous answer of mine related to the Singleton pattern in Android, in this case I would recommend janos' approach though.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.