Proper way to wait end of a service and singleton design

Question

My code works fine and I would like to know if I chose the best strategy to implement my code, or if It can written better (more fast, more clear, best use of design pattern, etc).

In my android application WelcomeActivity, in the onCreate method, I start a really simple service that load data from a file:

public class LoadDataService extends IntentService {
 private int result;
 @Override
 protected void onHandleIntent(Intent intent) {
 try 
 {
 FileTestSingleton.getInstance(); //this could be long, read from file
 result = Activity.RESULT_OK;
 } catch (Exception e) 
 {
 Log.e(LoadDataService.class.getName(), "FILE DATA LOADING ERROR \n" + e);
 result = Activity.RESULT_CANCELED;
 }
 publishResults(result);
 }
 private void publishResults(int result) {
 stopSelf();
 }
} 

Some activity after, the user click a button to go to the next step of my APP. The onCreate method of MyNewActivity need the data from the service, otherwise it's ok to freeze the application, because if the data aren't loaded, nothing can be done. For example, I read that to stop the UI thread is a bad thing, but I need to stop it, otherwise my new activity will not have data to show.

So in my onCreate:

while (FileTestSingleton.getState()==Semaphore.LOADING)
{
 Thread.sleep(250); 
}
fileTest = FileTestSingleton.getInstance();

Is this a proper way to wait?

Semaphore is a simple enum that I created inside the FileTestSingleton class:

public enum Semaphore
{
 EMPTY,
 LOADING,
 DONE;
}
private static Semaphore semaphore = Semaphore.EMPTY;
private static FileTestSingleton fileTest;
public static FileTestSingleton getInstance() throws Exception
{
 if (fileTest==null && semaphore!=Semaphore.LOADING)
 {
 semaphore = Semaphore.LOADING;
 fileTest = new FileTestSingleton(); //read from file, really slow
 semaphore = Semaphore.DONE;
 }
 return fileTest;
}
public static Semaphore getState()
{
 return semaphore;
}

Why I think that I can't use AsyncTask: The problem is: I load some txt files on the startup in the WelcomeActivity that is for 2 seconds on the screen. After the app go in the MiddleActivity and the user in the meantime can choose some options before he starts the final step of the application: FinalActivity. This FinalActivity need that all the txt files are loaded in the singleton.

Answer 1

Bug?

The catch block is a bit suspicious here:

try 
{
 FileTestSingleton.getIstance(); //this could be long, read from file
} catch (Exception e) 
{
 Log.e(LoadDataService.class.getName(), "FILE DATA LOADING ERROR \n" + e);
 result = Activity.RESULT_CANCELED;
}
result = Activity.RESULT_OK;
publishResults(result);

When you catch the exception, you set result = Activity.RESULT_CANCELED;, but execution continues and on the next line comes result = Activity.RESULT_OK;. In short, the method will return with success as if nothing happened.

Don't catch Exception

Always catch the most specific Exception type possible. By catching something overly generic, you may not notice when something truly unexpected is thrown. In most practical situations there is a more specific exception class that you should check.

Don't declare a method to throw Exception

A method that's declared to throw Exception is very poorly designed. It gives no clue to the caller about the kind of failure that can happen, and it's impossible to know how to recover from it. Surely you can think of something more specific, to give the chance to callers to handle the failure gracefully.

Coding style

There is a standard in Java for placing braces: the opening braces should be on the same line as their statement. Follow this style:

try {
 // ...
} catch (Exception e) {
 // ...
}

Proper way to wait for end of X

In other words, the proper way to execute a long operation without freezing the client. I think it's using an AsyncTask, with a ProgressDialog. Here's a rough sketch:

class WaitForServiceAsyncTask extends AsyncTask<Void, Integer, Void> {
 ProgressDialog mProgressDialog;
 protected void onPreExecute() {
 mProgressDialog = ProgressDialog.show(YourActivity.this, "",
 YourActivity.this.getString(R.string.msg_waiting_for_service), false, true);
 }
 protected Void doInBackground(Void... params) {
 // the long task -- calling the service
 }
}

This won't freeze the UI thread and will let the user know that the app is not dead, but actually doing something.

• Accollativo: Thanks, very precious! Yes, the first was a bug that I didn't notice. What about the Thread.sleep(250); Is it ok?
• janos: Yeah, that's not so good either. I added one more paragraph for that, using an AsyncTask. This is how I did before, and it worked very nicely.
• Accollativo: I cannot use AsyncTask because like I said, I have to start the file reading in the beginning activity of my app (soon as I can), and this WelcomeActivity is just a page with a logo that stay alive for 2 second before to go to the next activity (and the file reading is longer than 2 sec.).
• janos: I don't see how that prevents you from using an AsyncTask. Note that the AsyncTask I proposed will open a modal ProgressDialog which the user can't get out of. When the initialization is finished, the dialog will close, and you can transition to the next activity. The name "Async" is misleading here, because in fact the operation is very much synchronous. But instead of freezing the UI, it will show a progress dialog, which is a lot friendlier.
• Simon Forsberg: @Accollativo I am sure that it is still possible to use AsyncTask, you just need to use it correctly.

Answer 2

In addition to @janos suggestion about an AsyncTask (which is a good suggestion), there's also the approach of using Activity.runOnUiThread.

If your LoadDataService would have access to an instance of your Activity, then it could tell your activity to run some code on the UI thread, for example yourActivity.handleResults(result);

I've written a previous answer about using singleton pattern in Android.

Most often, Singleton pattern can be completely avoided.

In fact, I'm not convinced your use of Singleton pattern is very good. Can some other code call the constructor new FileTestSingleton(); ? If so, then it's not really a singleton.

About your getIstance(); method... What is an Istance? Did you mean Instance?

• toto2: You have to be careful with singletons in Android since they have a different behavior than Java. The Android runtime can kill and restart an application at any time and the singleton can therefore be randomly wiped out. It might not matter here, but if a singleton is used as an "in memory" database for example, it would be very bad.
• Accollativo: Thanks @toto2 you get another point. The singleton is used as an "in memory" DB. I didn't use a real DB, because my data are on various txt files, and I just need to read from it (I never write). @Simon like I replied to @janos, I can't use AsyncTask.