patternjavaMinor
Singleton class extending a parent class to utilise shared functionality
Viewed 0 times
utilisesharedparentsingletonclassfunctionalityextending
Problem
I have a singleton class which extends from an abstract java class. Two singleton classes extend from
Is this good design?
Abstract Parent Class
First Extended class
```
public class ContestantImageThreadManager extends ItemImageThreadManager{
private static ContestantImageThreadManager ref;
private ContestantImageThreadManager(){
super();
}
public static ContestantImageThreadManager getSingletonOb
ItemImageThreadManager, the reason for this is to use shared scheduling functionality. A thread is created and passed into the runThread method of ItemImageThreadManager.Is this good design?
Abstract Parent Class
public abstract class ItemImageThreadManager {
private Timer timer = new Timer();
public void runThread(final Thread runThread){
try {
scheduleTimer(runThread);
}
catch(IllegalStateException ise){
/**
* When the timer is cancelled (when screen closed) and the images are
* removed from memory(in-app resync) and this screen is opened again,
* an IllegalStateException will be thrown since the images will need
* to be re-downloaded
*
* Solution - create a new timer.
*/
timer = new Timer();
scheduleTimer(runThread);
}
}
/**
* Starts a new thread. This thread is run in the order is has been added.
* Since timer runs in it's own thread, this gaurantees that just one thread
* will be started at one time....
* @param thread
*/
private void scheduleTimer(final Thread thread){
timer.schedule(new TimerTask() {
public void run() {
thread.start();
try {
thread.join();
} catch (InterruptedException e) {
e.printStackTrace();
}
}
}, 0);
}
public void cancelTimer(){
timer.cancel();
}
}First Extended class
```
public class ContestantImageThreadManager extends ItemImageThreadManager{
private static ContestantImageThreadManager ref;
private ContestantImageThreadManager(){
super();
}
public static ContestantImageThreadManager getSingletonOb
Solution
As others mentioned the
You use the
Instead of catching
If you're not familiar with concurrent code check Java Concurrency in Practice.
getSingletonObject should be synchronized. There is a chapter in Effective Java, Second Edition about singletons (Item 3: Enforce the singleton property with a private constructor or an enum type). It's worth to read. Anyway, try to avoid them, they makes testing really hard.You use the
Timer as a single-threaded executor. In that case consider using Executors.newSingleThreadExecutor. Furthermore, are you sure that you need to pass Thread at all? Wouldn't Runnable be enough? Executors can run Runnables without any wrapper class and Thread.join call.Instead of catching
IllegalStateException in the runThread method you should check whether the timer is canceled. You can store this state in a boolean flag. Don't forget to synchronize the variable access. (See Effective Java, Item 57: Use exceptions only for exceptional conditions)ExecutorService.submit returns Future which has a cancel method. Maybe it's better for you requirements (instead of Timer.cancel()).If you're not familiar with concurrent code check Java Concurrency in Practice.
Context
StackExchange Code Review Q#6431, answer score: 2
Revisions (0)
No revisions yet.