Pull requests

#14 Open
Repository
kevglass kevglass
Branch
development

fix Shape.contains not using working

Bitbucket cannot automatically merge this request due to conflicts.

Review the conflicts on the Overview tab. You can then either decline the request or merge it manually on your local system using the following commands:

hg update development
hg pull -r golvok/fix-shapecontains-not-working-1372039819667 https://bitbucket.org/golvok/slick
hg merge golvok/fix-shapecontains-not-working-1372039819667
hg commit -m 'Merged in golvok/slick/golvok/fix-shapecontains-not-working-1372039819667 (pull request #14)'
Author
  1. Matthew Walker
Reviewers
Description

fix Shape.contains not using intersects properly

Comments (4)

  1. nguillaumin

    The javadoc says "Check if the shape passed is entirely contained within this shape", so wouldn't the current logic be correct, since if 2 shapes intersect one can't be entirely contained within the other?

  2. nguillaumin

    Actually I think this test should be removed, because there are actually 2 cases:

    • Shape are only partially intersecting: contains() should return false
    • One shape is completely intersected (contained) within the other: contains() should return true

    But intersects() doesn't distinguish between partial or complete intersection, so it can't be use to take an early exit.

    Thoughts?

  3. Matthew Walker author

    I think that was what I realized at the time.

    I also think I thought it was a typo, and that I assumed that it was meant to be an early out because it is a faster (maybe?) condition to check.