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

line(a, b) vs lineTo([a, b]) argument types #41

Closed
lf94 opened this issue Jun 30, 2022 · 5 comments
Closed

line(a, b) vs lineTo([a, b]) argument types #41

lf94 opened this issue Jun 30, 2022 · 5 comments

Comments

@lf94
Copy link

lf94 commented Jun 30, 2022

How come there's this difference?

@sgenoud
Copy link
Owner

sgenoud commented Jun 30, 2022

This is the different between an absolute and a relative position. When you draw (or sketch) your pointer is at a position (for instance you drew a line to [5, 2]. If you do line(1, 1) you will draw to the point [6, 3]. If you do lineTo([1, 1]) you will draw to [1, 1]

@lf94
Copy link
Author

lf94 commented Jun 30, 2022

I should've been more explicit... Why are the arguments not the same types? Sorry 🥲

@lf94 lf94 changed the title line(a, b) vs lineTo([a, b]) line(a, b) vs lineTo([a, b]) argument types Jun 30, 2022
@sgenoud
Copy link
Owner

sgenoud commented Jun 30, 2022

This tries to make explicit that these two actions are different.

When you move absolutely you move to a point (i.e. an array of two numbers). When you move in relative you change distances (which is not a point).

I concede that when you move relatively you move by a vector (which is also an array of numbers). Is there a reason you would prefer to have it as a vector?

@lf94
Copy link
Author

lf94 commented Jun 30, 2022

It's a more consistent API interface - when running svg2replicad I didn't realize this and was getting an error - it was because I presumed they were of the same type!

It makes calling more consistent: line(x, y) vs lineTo(v) is inconsistent when they are both drawing lines... And there are probably cases for when you want to do math on the absolute point v that transforms it into a "relative point" v2 and vice-versa... The math then only involves 1 type, making everything easier to understand (instead of seeing additional type conversion)

You could keep the old API for line and just provide it an overloaded type as well (line(x: number | Point2D, y?: number))

@sgenoud
Copy link
Owner

sgenoud commented Jul 1, 2022

The math then only involves 1 type, making everything easier to understand (instead of seeing additional type conversion)

It feels like it would be less welcoming for new users. It makes the API more consistant - but the action behind it is not consistant. In some ways it exposes the inconsistency (line and lineTo are not the same thing) in a way that aligns with what it does.

The math then only involves 1 type, making everything easier to understand (instead of seeing additional type conversion)

Note that the conversion is not that complicated to do line(...vector) should just work.

There is a bit of inconsistency with the way I do the translate - sometimes I take a vector sometimes a list of distances. I will support everywhere the list of distances for consistency with the "what is absolute and what is relative". I will keep the type overloaded for backwards compatibility but might remove it at some point in the future.

I will close this issue for now - if I see it coming up again and again with newbies I will reconsider!

@sgenoud sgenoud closed this as completed Jul 1, 2022
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

No branches or pull requests

2 participants