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

Refactor rectangle and box types #41231

Merged
merged 2 commits into from
Jun 12, 2020

Conversation

jbytheway
Copy link
Contributor

Summary

SUMMARY: Infrastructure "Refactor rectangle and box types"

Purpose of change

Previously rectangle and box were just a min and max. Determining whether the upper bound should be considered to be inclusive was done by which function was called on it.

I now regret that design choice, and am changing it.

Describe the solution

Now the inclusive vs half-open is part of the rectangle or box type, and the functions are generic.

This leads to less repetition in the code.

Also, the next step is to make it possible to iterate over rectangles (to simplify a bunch of our nested for loops), and this design fits much better for that.

Describe alternatives you've considered

Leaving as-is.

Testing

Unit tests.

Additional context

Indirectly part of the effort towards #32017.

jbytheway and others added 2 commits June 11, 2020 09:32
Previously rectangle and box were just a min and max.  Determining
whether the upper bound should be considered to be inclusive was done by
which function was called on it.

I now regret that design choice, and am changing it.  Now the inclusive
vs half-open is part of the rectangle or box type, and the functions are
generic.

This leads to less repetition in the code.

Also, the next step is to make it possible to iterate over rectangles
(to simplify a bunch of our nested for loops), and this design fits much
better for that.
@ZhilkinSerg ZhilkinSerg merged commit 4796a02 into CleverRaven:master Jun 12, 2020
@jbytheway jbytheway deleted the rectangle_refactoring branch June 12, 2020 12:25
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.

3 participants