HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Multi-threaded Command Processor

Submitted by: @import:stackexchange-codereview··
0
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

Solution

Getting state

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.