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

Update Conditional Logic for Grade Assignment #672

Closed
wants to merge 1 commit into from

Conversation

Shivay-Shakti
Copy link

Summary

This pull request updates the conditional logic used for assigning letter grades based on numerical scores.

Changes

  • Changed the condition for a 'C' grade to check for scores between 70 and 79 inclusive.
  • Updated the condition for a 'B' grade to check for scores between 80 and 89 inclusive.

Motivation

The previous logic incorrectly assigned a 'C' grade to any score above 70 and a 'B' grade for any score above 80, without upper bounds. This update corrects the grade assignment to reflect the standard grading scale.

Testing

  • Added test cases for the boundary conditions to ensure the grades are assigned correctly.
  • Ran the entire test suite to confirm that no existing functionality is broken.

Greetings Team, 

During one of our demos, it was brought to our notice that the conditional statement and output are incorrect. I have updated the conditional statement to cover all the cases.
Copy link

🆗 Pre-flight checks passed 😃

This pull request has been checked and contains no modified workflow files, spoofing, or invalid commits.

It should be safe to Approve and Run the workflows that need maintainer approval.

@alee
Copy link
Member

alee commented Feb 16, 2024

Thanks for taking the time to submit this PR! @sborrego submit the PR right before this that also addresses this issue in a simpler way with less conditional checks so I think we will go with that one. I'm guessing you all caught this during a carpentries workshop?

Thanks again, we appreciate your contributions!

@alee alee closed this Feb 16, 2024
@Shivay-Shakti
Copy link
Author

That's amazing, yes we found it during the workshop. Thanks, @sborrego and @alee for your pro-activeness!

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.

2 participants