Skip to content

TargetState#63

Merged
alicekuznetsov merged 16 commits intomainfrom
targetstate
Sep 21, 2024
Merged

TargetState#63
alicekuznetsov merged 16 commits intomainfrom
targetstate

Conversation

@alicekuznetsov
Copy link
Collaborator

@TaylerUva @Alenguye582

bros...
While working on this, I thought that a HashMap with the desired TargetState would be the cleanest way of storing the preset values, but I wasn't sure how we'd want to store those groups of measures in it.
I could think of 3 implementations I enjoyed but I'm not sure which one is the best 😀. Ultimately, it doesn't matter, as I'll just write a shooter method that accepts whatever we pick [such as setShooterToPreset(Matrix<N3, N1>)] but I think we'll run into this issue later if we continue using Measures

  1. Measure array size isn't specified
    public static Map<TargetState, Measure[]> TARGET_TO_PRESETS;

  2. 2 sets of pairs (tuples aren't in our IDE and I couldn't find an alternative, let me know if you do bc i would love that)
    public static Map<TargetState, Pair<Measure<Angle>, Pair<Measure<Velocity<Angle>>, Measure<Velocity<Angle>>>>> TARGET_TO_PRESETS;

  3. Convert the Measures to doubles and store them in a Matrix
    public static Map<TargetState, Matrix<N3, N1>> TARGET_TO_PRESETS;

(I also considered making a separate object like public ThreeSpecificMeasures(Measure<Angle>[] shooterAngle, Measure<Velocity<Angle>> leftVelocity, Measure<Velocity<Angle>> rightVelocity) and using that as the key value but I figured that would be excessive)

@TaylerUva
Copy link
Member

TaylerUva commented Sep 17, 2024

I like 'public static Map<TargetState, Measure[]> TARGET_TO_PRESETS;' however then it is on the user to make sure the order is correct. Object idea prevents that specific issue at the cost of some increased complexity.

I'm leaning toward Object as my #1 and Measure array as #2

We could also make 3 maps: 1 for angle one for left and one for right which may be the simplest implementation?

@alicekuznetsov
Copy link
Collaborator Author

I don't like the idea of 3 separate maps just for the sake of keeping them together.
I'll go with the object implementation :)

@alicekuznetsov alicekuznetsov marked this pull request as ready for review September 21, 2024 02:00
@alicekuznetsov alicekuznetsov requested a review from a team as a code owner September 21, 2024 02:00
@alicekuznetsov
Copy link
Collaborator Author

Closes #59

@alicekuznetsov alicekuznetsov enabled auto-merge (squash) September 21, 2024 02:03
@alicekuznetsov alicekuznetsov merged commit 50bfa73 into main Sep 21, 2024
@alicekuznetsov alicekuznetsov deleted the targetstate branch September 21, 2024 02:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants