Skip to content

Add additional tests to rotate functions, clarify variable names #75

New issue

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

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

Already on GitHub? # to your account

Merged
merged 4 commits into from
Feb 16, 2021

Conversation

mdznr
Copy link
Contributor

@mdznr mdznr commented Feb 10, 2021

  • Adds specific unit tests for the documented examples of the rotate functions. It’s good to specifically test the examples in the documentation. After all, those are the cases that are absolutely expected to work.
  • Adds unit tests for _reverse(subrange:until:) and reverse(subrange:)
  • Within documentation and implementation of _reverse(subrange:until:), use lower and upper instead of f and l. l could easily be misinterpreted as lower, left, or limit. Using lower and upper avoids this ambiguity.
  • Use consistent variable names lower and upper instead of lo, and hi, respectively, in reverse(subrange:). This is more consistent with lowerBound and upperBound.

Checklist

  • I've added at least one test that validates that my change is working, if appropriate
  • I've followed the code style of the rest of the project
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary

@mdznr
Copy link
Contributor Author

mdznr commented Feb 15, 2021

Thanks for the review, @natecook1000. I’ve updated the code and I believe it reflects all the feedback given.

@natecook1000
Copy link
Member

LGTM! Thanks @mdznr 🕺

@natecook1000 natecook1000 merged commit 5be792a into apple:main Feb 16, 2021
@mdznr mdznr deleted the rotate_tests branch February 16, 2021 15:44
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants