patternjavaMinor
Multi-threaded Command Processor
Viewed 0 times
processorcommandthreadedmulti
Problem
I have written a class that is intended to execute a unit of work to be run in a separate thread. The intended use case for this class is running business logic off of the user interface thread so as not to lock it up while waiting for long running processes to finish.
Here is the class:
```
import javafx.beans.property.ObjectProperty;
import javafx.beans.property.ReadOnlyObjectProperty;
import javafx.beans.property.SimpleObjectProperty;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Callable;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
public abstract class Command implements Callable {
private static final Logger logger = LoggerFactory.getLogger(Command.class);
private final Map>> stateListeners = new ConcurrentHashMap<>();
private final ObjectProperty state = new SimpleObjectProperty<>();
private final ObjectProperty value = new SimpleObjectProperty<>();
private final ObjectProperty exception = new SimpleObjectProperty<>();
private Thread thread;
public Command() {
reset();
state.addListener((observable, oldValue, newValue) -> {
logger.trace(this.getClass().getSimpleName() + " changed state to " + newValue);
});
}
public boolean isReady() {
return state.get().equals(State.READY);
}
public boolean isRunning() {
return state.get().equals(State.RUNNING);
}
public boolean isDone() {
switch (state.get()) {
case SUCCEEDED:
case FAILED:
case CANCELLED:
return true;
default:
return false;
}
}
public boolean isSucceeded() {
return state.get().equals(State.SUCCEEDED);
}
public boolean isFailed() {
return state.get().equals(State.FAILED);
}
public boolean isCancelled() {
return state.ge
Here is the class:
```
import javafx.beans.property.ObjectProperty;
import javafx.beans.property.ReadOnlyObjectProperty;
import javafx.beans.property.SimpleObjectProperty;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Callable;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
public abstract class Command implements Callable {
private static final Logger logger = LoggerFactory.getLogger(Command.class);
private final Map>> stateListeners = new ConcurrentHashMap<>();
private final ObjectProperty state = new SimpleObjectProperty<>();
private final ObjectProperty value = new SimpleObjectProperty<>();
private final ObjectProperty exception = new SimpleObjectProperty<>();
private Thread thread;
public Command() {
reset();
state.addListener((observable, oldValue, newValue) -> {
logger.trace(this.getClass().getSimpleName() + " changed state to " + newValue);
});
}
public boolean isReady() {
return state.get().equals(State.READY);
}
public boolean isRunning() {
return state.get().equals(State.RUNNING);
}
public boolean isDone() {
switch (state.get()) {
case SUCCEEDED:
case FAILED:
case CANCELLED:
return true;
default:
return false;
}
}
public boolean isSucceeded() {
return state.get().equals(State.SUCCEEDED);
}
public boolean isFailed() {
return state.get().equals(State.FAILED);
}
public boolean isCancelled() {
return state.ge
Solution
Getting state
Since you already have a
Comparing
Enums are safe for comparison by
Comparing
If you are comparing against a set of
public State getState() {
return state.get();
}Since you already have a
getState() method, you can consider using it instead of state.get().Comparing
State enum (part one)Enums are safe for comparison by
==, so most of your operations on equals() can be replaced accordingly.Comparing
State enum (part two)If you are comparing against a set of
enum values, such as in your isDone() method, you can also consider using EnumSet.contains() instead of the switch statement:Set DONE_STATES = EnumSet.of(State.SUCCEEDED, State.FAILED, State.CANCELLED);
public boolean isDone() {
return DONE_STATES.contains(getState());
}Code Snippets
public State getState() {
return state.get();
}Set<State> DONE_STATES = EnumSet.of(State.SUCCEEDED, State.FAILED, State.CANCELLED);
public boolean isDone() {
return DONE_STATES.contains(getState());
}Context
StackExchange Code Review Q#114923, answer score: 4
Revisions (0)
No revisions yet.