Hi, I’m one of the programming leads on FRC team 4500. I’ve recently been looking into in-lining commands as opposed to sub-classing them, which is what we’ve always done. I saw that a major factor dissuading people from using inline commands is that storing information across scheduler runs is very difficult (changing some variable in the execute method and using the changed variable in the next execute call).
I came up with an idea for a potential solution, but I wanted someone who is more experienced in java to read it first (The only experience I have is one build season of FRC) and see if it would work and isn’t hot garbage.
public Command exampleCommand() {
/* To store data, you would create a class with the variables
you want to store */
final class DataHolder {
public double exampleDouble = 0;
}
/// Then you could create an instance of this class
DataHolder dataHolder = new DataHolder();
return Commands.run(
() -> {
// And then you could do something with the variables
System.out.println(dataHolder.exampleDouble);
dataHolder.exampleDouble++;
}
);
}
My two main questions about this were:
Would it work?
Is it good quality wise, or would it make an experienced java coder throw up?
Any thoughts/feedback/discussion would be greatly appreciated!
It’s the best way to do what you need to. Note that the effectively-final restriction means that it’s much harder to do state as primitives than in mutable objects, which is why your solution needs the wrapper class. If you’re capturing other objects, they don’t need to go in the DataHolder. I’m not sure what kind of effect declaring the class inside the function has, but you wouldn’t be calling this much.
If you’re just trying to get around the “cannot refer to a non-final variable” requirement, you can use AtomicReference, even though it sorta violates the intent of that class.
public Command exampleCommand() {
final AtomicReference<Double> exampleDouble = new AtomicReference<>(0.0);
return Commands.run(
() -> {
double currentValue = exampleDouble.get();
System.out.println(currentValue);
exampleDouble.set(currentValue + 1.0);
}
);
}
It sorta seems like you’re sorta reshaping the problem to fit a preferred solution. If you look at the source for Commands, the static methods are just syntactic sugar. A constructor is going to be called for a command class at some point. I love that sorta thing too, though so you can replicate it yourself like this:
public final class MyCommand extends RunCommand {
private double exampleDouble = 0.0;
private MyCommand(Runnable toRun, Subsystem... requirements) {
super(toRun, requirements);
}
@Override
public void execute() {
super.execute();
System.out.println(exampleDouble);
exampleDouble++;
}
public static Command run(Runnable action, Subsystem... requirements) {
return new MyCommand(action, requirements);
}
}
My solution to this is to make the subsystem have the vast majority of the intelligence, and be more stateful, rather than try to add logic to the command.
Nothing really preventing me from doing this, I just wanted to see how much you could do with in-lining commands alone. I’d agree that in most situations where a command needs a state, storing that state in a subsystem or subclassing that command are the best options.
If you do use the pattern from your OP, State is probably a better name than DataHolder.
Wrapping primitive state in containers to allow mutable capture is a common Java pattern, so you’re not doing anything an experienced developer wouldn’t recognize there.