Line does not implement equality operator

[kigster]

I think it's pretty crucial that identical lines can be identified. In my project it is essential to be able to sort and dedup arrays of lines, and I can not imagine doing this without equality operator.

irb(main):003:0> l1 = Geometry::Line[[1,0], [10,10]]
=> Line(Point[1, 0], Point[10, 10])
irb(main):004:0> l2 = Geometry::Line[[1,0], [10,10]]
=> Line(Point[1, 0], Point[10, 10])
irb(main):005:0> l1.eql?(l2)
=> false
irb(main):006:0> l1 == l2
=> false

[bfoz]

As a short term workaround, have you tried using Edges instead of Lines? Edge already has the == operator.

This is definitely something that needs to be added, but I'm not sure how to handle comparisons between different sub-classes of Line. For example, should a PointSlopeLine and a TwoPointLine always compare unequal, even if they happen to be the same line? Even if the comparison does the math to see if the two line types are in fact equivalent, floating point issues may still impact the result. Any thoughts?

[bfoz]

I just added some braindead comparison operators to the Line cluster. This needs some more thought, but hopefully it will work well enough for the moment. Let me know how it goes.

[kigster]

Well, equality between points is simple both x and y should be the same.

I think the same applies for lines: their start and end points should be the same. I don't think it should depend on the specific subclass implementing the line, because the subclasses are used simply as a factory to create lines in different ways.

Another interesting question is whether the directionality is factored into the equality: meaning, if lines are the same, but one lines start is the other line's end. The way I would address this is as follows:

# Line's equality is implemented without regard to direction, as long as if you drew the lines they would look the same on 2D plane, they are the same.
# But for directional line, additional requirement is that they match in start/end.
class DirectionalLine < Line 
def eql?(other) 
  other.is_a?(DirectionalLine) && self.first.eql?(other.start) && self.last.eql?(other.last)
end

I started looking at your library because I needed a lot of these primitives for my gem called laser-cutter: https://github.com/kigster/laser-cutter

I implemented Line, Point, Rectable primitives, but not in the most clean way. I did this before I discovered your library. Do you accept pull requests? If I move over some of the functionality from the laser-cutter gem to this gem's primitives, such as ordering, equality, hashing, shifting the position, etc, would that be something you'd consider accepting?

If so, my last request would be to integrate rspec into the project, and write new tests in rspec. It's just so much nicer, and a lot of ruby developers are more familiar with rspec than minitest.

Thanks!

[bfoz]

Actually, the Line subclasses aren't equivalent. They are different implementations of the concept of a line. For instance, PointSlopeLine stores the line's slope and a single point on the line. Whereas TwoPointLine represents a line by storing two points that lie on the line (but doesn't store the slope). The Line class provides a uniform interface to the subclasses so that users don't need to worry about the implementation details.

The Line class cluster is meant to represent infinitely long lines, and is therefore directionless. Of course, the subclasses that store the line's slope are implicitly directional. I'm not yet sure how to handle that for comparisons.

Of course I'll consider any pull requests. I won't promise to accept all of them, but I'll definitely look at them.

I took a quick look at your code, and I think geometry will do most of what you need. For the few cases it doesn't, it might be quickest to reopen the classes and add what's missing. Once we have a better idea of the changes that geometry needs then we can talk about that. But that's a discussion for another issue.

In the short term, you might want to look at using the Edge class, rather than Line. I think it's closer to what you need.

The project is only using minitest because it's built in. I don't have much need for it to be one way or the other. Regardless, that's a discussion for another thread.

[kigster]

Right, I guess it all depends on whether you consider equality in the true geometric sense, or more specifically based on how it's implemented internally.

To me it seems that, given that the gem's name is just that – geometry – it would be slightly more appropriate to implement equality based on mathematical terms, not implementation details. In other words, if you have a line with a beginning and an end, and another line between the same two points, these two lines should be equal to each other, regardless of how they are implemented internally. Isn't that the whole point of OO, to hide implementation?

Lines that are infinite should probably also be equal if and only if their geometrical representations are equal (so they overlap completely and entirely in a euclidean plane).

It's very nice to be able to create lines using many different methods, and I like that you provide radians and other means of doing so, but those just seem like constructors to me, representing ultimately the same object.

In the end it all depends on what meaning you prefer to attach to the concept of a line in your codebase. I made an assumption that it was based on the pure geometry, and I suspect others will too.

My 2c.

[bfoz]

Your assumption is correct. Geometry's primitives are meant to be representations of pure geometric concepts, with the implementation details hidden when appropriate. However, the code has been developed 'as needed', so it sometimes strays from geometric purity.

I've pushed some code that should enable cross-subclass comparisons for the Line class cluster. Give it a try and let me know if I missed something. Feel free to open a new issue if there are any bugs.