Skip to content

Update for GEllipse#1

Closed
akevalion wants to merge 7 commits intopharo-contributions:masterfrom
akevalion:master
Closed

Update for GEllipse#1
akevalion wants to merge 7 commits intopharo-contributions:masterfrom
akevalion:master

Conversation

@akevalion
Copy link
Copy Markdown

Added

  • asPoint for GPoint
  • fix GEllipse intersectionsWithLine: for empty ellipses
  • Added class comment for GEllipse
  • GRectangle now accepts empty rectangles
  • remove manifest package tag

- asPoint for GPoint
- fix GEllipse intersectionsWithLine: for empty ellipses
- Added class comment for GEllipse
- GRectangle now accepts empty rectangles
- remove manifest package tag
Comment thread src/Geometry/GEllipse.class.st Outdated
or := (point1 x min: point2 x) , (point1 y min: point2 y).
cor := (point1 x max: point2 x) , (point1 y max: point2 y).

or = cor ifTrue: [ self error: 'This is not a rectangle but a point.' ].
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which case do you want a rectangle that is a point?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked and from the mathematical point of view a point cannot be considered as a rectangle.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is your reference? From my perspective, users can have empty shapes like an empty ellipse or empty rectangles, or empty polygons.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked multiple sources such as Wikipedia, Wolfram, the dictionary...

All of them agree on "quadrilateral with opposite sides of equal lengths a and b, and with four right angles"

A point cannot have right angles. Thus, a point is not a rectangle.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rollback this method to its previous version

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For empty shapes, I would create a NullShape. For intersections, for example, it would always return an empty collection.

Or if your empty shape acts as a point, then I would use a point instead. IIRC points can react to #intersections:

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will use points for that then.

]

{ #category : #tests }
GEllipseTest >> testEmpty [
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, so the code managed ellispes without vertexes.

I think we are in the same case of the rectangle, those are not ellispses.

An ellipse is a curve that is the locus of all points in the plane the sum of whose distances r_1 and r_2 from two fixed points F_1 and F_2 (the foci) separated by a distance of 2c is a given positive constant 2a (Hilbert and Cohn-Vossen 1999, p. 2).

An ellipse needs a curve, which is not possible when there are no vertexes :(

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For roassal when shapes were empty I will use points

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the constructors of GRectangle and GEllipse can return a point instead of an error

@akevalion akevalion closed this Aug 27, 2020
jecisc pushed a commit that referenced this pull request Mar 12, 2026
Update distanceTo: for GSegment
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