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

do not resize window that overlaps with current window. #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wengxt
Copy link

@wengxt wengxt commented Apr 19, 2016

Fix #2

@Flupp
Copy link
Owner

Flupp commented Jun 6, 2017

Thanks for your contribution! Please excuse my late response.

I do not quite understand your conditions in rectInteresects. For example in line 89: why do you add 1 on the left side of the comparison? Why is it not just rt1.x1 > rt1.x2?

Also I find the code quite difficult to understand, since you first assign both l1 and r1 the same value and change one of them afterwards depending on the condition. I would find it easier to understand, if both variables were assigned after evaluating the condition.

If adding 1 (see above) is wrong in the end, the code would boil down to l1 = min(rt1.x1, rt1.x2); r1 = max(rt1.x1, rt1.x2);. But then also the potential swapping of .x1 and .x2 would never occur, because it is already guaranteed on the calling side that .x1 <= .x2.

Why is the function called rectIntersects and not, e.g., rectsIntersect? Since two rects are checked for intersection I would expect plural.

Last, but not least, please use tabs for indentation like in the rest of the code.

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