2
\$\begingroup\$
public static ArrayList<Playlist> getAllPlaylists( Context context ){
    ArrayList<Playlist> playlists = new ArrayList<Playlist>();
    ArrayList<String> fileList = new ArrayList<>();

    //this will get me all playlist files available for my application
    for( String file : context.fileList() )
        if( file.endsWith(".playlist") )
            fileList.add(file);

    for( String file : fileList ){
        Playlist newPlaylist = loadPlaylist( context, file );

        if( !newPlaylist.isEmpty() ){
            playlists.add(newPlaylist);
        }
    }

    return playlists;
}

private static Playlist loadPlaylist( Context context, String file ){
    Playlist playlist = new Playlist();

        try {
            FileInputStream inputStream = context.openFileInput(file);
            byte[] inputBuffer = new byte[ inputStream.available() ];
            inputStream.read(inputBuffer);
            String content = new String(inputBuffer);
            //each line holds a path so we split the content
            String[] files = content.split("\n");

            playlist.setName( files[0] );               
            for( int i = 1; i < files.length; i++ ){
                //This will just load the Song (artist, title etc.)
                //if somehow the path leads to nowhere or it is just not a 
                //music file, getSong() will return null
                Song song = MediaManager.getSong( context, files[i] );

                if( song != null )
                    playlist.addSong( song );
            }
        } catch (FileNotFoundException e) {
            e.printStackTrace();
        } catch (IOException e) {
            e.printStackTrace();
        }

    return playlist;
}

The structure I chose for my playlist files is pretty simple. The first line is the name of that playlist and the following lines will contain a filepath which I can use later on to load a file and play it / display its data.

Here is an example of what they might look like :

My first Playlist
/sdcard/.../.../whatever.mp3
/sdcard/.../.../noway.mp3
/sdcard/.../.../seriously.mp3

How can I improve the speed of this process? It takes about 1 second for 1 file...

\$\endgroup\$
  • \$\begingroup\$ Nitpicking: it is legal to have a newline in path. You may want to extract filenames in a more diligent way. \$\endgroup\$ – vnp Jul 30 '14 at 22:32
  • 1
    \$\begingroup\$ @vnp what?! damn... thanks \$\endgroup\$ – Davlog Jul 30 '14 at 22:36
  • \$\begingroup\$ If you aren't already, make sure to use AsyncTask so that the operation can be performed in the background without making your application not responding. \$\endgroup\$ – Simon Forsberg Jul 31 '14 at 0:05
5
\$\begingroup\$

About the slowness...

I don't see obvious bottlenecks here. When you say "It takes about 1 second for 1 file..." do you mean:

  • A single song? In this case the MediaManager.getSong is a prime suspect. Add logging before and after to measure the difference.
  • An entire playlist file?
    • Are there many songs in a list?
    • Does it make a difference if all the files in the list are valid or invalid? Test with 100 valid files and 100 invalid files. If invalid files somehow cause MediaManager.getSong to be slow, you can try to add a check if the file exists before passing it to MediaManager.

These are mostly shots in the dark.

There are some other problems with your code that won't make your program faster but you should fix anyway.

Reading from streams

The documentation says this about inputStream.available():

Returns an estimated number of bytes that can be read or skipped without blocking for more input.

Note that this method provides such a weak guarantee that it is not very useful in practice.

Don't use it this way. Read like this instead:

FileInputStream inputStream = context.openFileInput(file);
InputStreamReader inputStreamReader = new InputStreamReader(inputStream);
BufferedReader bufferedReader = new BufferedReader(inputStreamReader);
String line;
List<String> files = new ArrayList<String>();
while ((line = bufferedReader.readLine()) != null) {
    files.add(line);
}
bufferedReader.close();

Iterator<String> iter = files.iterator();
playlist.setName(iter.next());
while (iter.hasNext()) {
    String path = iter.next();
    Song song = MediaManager.getSong(context, path);
    if (song != null) {
        playlist.addSong(song);
    }
}

An important mistake you did there that you didn't close the input stream. Remember to close resources that you opened.

Use interface types instead of implementations

Instead of this:

ArrayList<Playlist> playlists = new ArrayList<Playlist>();

Use the interface type in declarations, like this:

List<Playlist> playlists = new ArrayList<Playlist>();

Style issues

This code doesn't follow the standard spacing rules, and the convention of using braces even for one-line statements:

for( String file : context.fileList() )
    if( file.endsWith(".playlist") )
        fileList.add(file);

This is the recommended style:

for (String file : context.fileList()) {
    if (file.endsWith(".playlist")) {
        fileList.add(file);
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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