Shenzhen-Robotics-Alliance / maple-sim

Elevating FRC Java Robot Simulations to the Next Level with Physics Engines
BSD 3-Clause "New" or "Revised" License
20 stars 13 forks source link

[API Enhancement] Dependency Injection For Intake Simulation #6

Closed oh-yes-0-fps closed 4 weeks ago

oh-yes-0-fps commented 4 weeks ago

Description

Dependency injection For IntakeSimulation instead of inheritance.

Current Code

public class IntakeSim extends IntakeSimulation implements Intake {

  public IntakeSim(String targetedGamePieceType,
      AbstractDriveTrainSimulation driveTrainSimulation,
      double width,
      IntakeSide side,
      int capacity) {
    super(...);
  }

  public void runIntake(double voltage) {
    if (voltage < 0.0) {
      startIntake();
    } else {
      stopIntake();
    }
}

Suggested Improvement

public class IntakeSim implements Intake {
  private final IntakeSimulation simulator;

  public IntakeSim(IntakeSimulation simulator) {
    this.simulator = simulator;
  }

  public void runIntake(double voltage) {
    if (voltage < 0.0) {
      simulator.startIntake();
    } else {
      simulator.stopIntake();
    }
}

Implementation

Simply removing the abstract keyword from the class def could work here.

Justification

Requiring the user to extend IntakeSimulation when the class has no abstract methods is an odd decision. I believe it would be more intuitive and flexible simply passing in an IntakeSimulation object, this would also allow teams that aren't doing hardware abstraction to utilize this feature(maybe using Optional or something). Finally, having the library methods associated with a specific object and not something you are inheriting from makes it easier to track what functions are user code and what functions are the library.

Additional

Current workarounds

Right now you can just make an anonymous class and dependency inject that: var simulator = new IntakeSimulation( ... ) {};

Comments

The dependency injection argument can be made for the whole library to be honest, but the abstract class with no abstract methods is what stuck out to me here.

catr1xLiu commented 4 weeks ago

I've added two public methods to allow users to use IntakeSimulation in the dependency injection style. The documents are also updated. https://github.com/Shenzhen-Robotics-Alliance/maple-sim/commit/9883f4125bc9a43ea3b1ee3acb003a8250337f7c