Make button bindings more concise #168
Conversation
I guess this works because `RobotContainer.kt` and `Visualizer.kt` are in the same package.
We use the REV's PDH, not CTRE's PDP. However, the default is to detect the type automatically, so I removed all parameters.
Qodana Community for JVM239 new problems were found
☁️ View the detailed Qodana report Contact Qodana teamContact us at [email protected]
|
| operatorController | ||
| .povUp() | ||
| .onTrue(extender.reset(operatorController.povUp().negate())) | ||
| driverController.apply { |
There was a problem hiding this comment.
I think this is less readable and makes it harder to switch buttons between controllers
There was a problem hiding this comment.
Perhaps it's for the better, otherwise there's more chance for errors. The biggest improvement here is that you don't need to write the button twice (the negate part is done for you).
It won't be hard to switch buttons, you just need to remove one line and add another in the second section. You can refactor the mapping into a constant/function if you'd like.
IMO the previous code too verbose, and it was easy to miss stuff in all the boilerplate.
There was a problem hiding this comment.
I like not having to write the negate but I still think this is less readable, maybe an extension function could be good.
There was a problem hiding this comment.
What would that exentsion function look like?
What exactly about the code looks unreadable?
@rakrakon, what is your opinion?
There was a problem hiding this comment.
First of all I agree with Emma about the readability. As for the code duplication I think our best option is to do something like this:
driverController.circle().run {
onTrue(l3(negate()))
}
There was a problem hiding this comment.
If anything, I think that using the negate without the object is less readable than not specifying the controller. My concern is that because the controller name is written so many times we will glance over the code and miss important things.
I don't really see how not specifying the controller hinders readability. What do you think about extracting the code for each controller into its own function
In any case, I don't think I'm going to add code to this branch so you can just push your suggestion and then we'll see how it looks.
And other small changes.
I DIDN'T TEST THIS (because my computer is a potato) SO PLEASE CHECK THE CODE. Thanks :)
visualizerinstantiation with declarationifstatementLoggedPowerDistributionmodule type