Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #14, Refactor LC_SampleAPs to remove extraneous if statement #47

Merged
merged 1 commit into from
Apr 20, 2023

Conversation

thnkslprpt
Copy link
Contributor

@thnkslprpt thnkslprpt commented Nov 1, 2022

Checklist

Describe the contribution

  • Fixes Refactor LC_SampleAPs #14
    • LC_SampleAPs() has been refactored to remove the extraneous if statement.
    • Action points are still sampled in the same way, and if StartIndex == EndIndex, the for loop will only run once (i.e. the same as before).

Testing performed
GitHub CI actions (incl. Build + Run, Unit Tests etc.) all passing successfully.

Expected behavior changes
Intent of the code remains basically unchanged, although the logic has changed slightly.
Main change is that CurrentAPState is checked as valid before any sampling of action points, whereas previously this was only checked in the single action point sample condition - i.e. if (StartIndex == EndIndex)

Contributor Info
Avi @thnkslprpt

@dzbaker dzbaker requested a review from chillfig November 3, 2022 18:31
@dzbaker dzbaker added this to the Fornax milestone Nov 21, 2022
@dzbaker dzbaker modified the milestones: Fornax, Equuleus Dec 7, 2022
Copy link
Contributor

@chillfig chillfig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removes extraneous logic. Looks good!

@dzbaker dzbaker merged commit 2dd1c7d into nasa:main Apr 20, 2023
@thnkslprpt thnkslprpt deleted the fix-14-refactor-LC_SampleAPs branch April 20, 2023 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor LC_SampleAPs
3 participants