servo / euclid

Geometry primitives (basic linear algebra) for Rust
Other
462 stars 102 forks source link

Add getters providing strongly typed Length values #461

Open nical opened 4 years ago

nical commented 4 years ago

Fixes #396 and #388.

This PR adds strongly typed Length getters with a simple naming scheme: anything prefixed with get_ returns a Length<T, U>, while unprefixed methods returning a scalar and direct scalar member accesses yield T directly.

The main motivations behind this approach are:

nical commented 4 years ago

There are a few other getters to add but I'd like to discuss the naming scheme before doing the rest of the work.

nical commented 4 years ago

For reference, I tinkered in a separate branch with another naming scheme that avoids the get_ prefix:

That scheme looks nice in principle, in practice it make the ergonomics of working with scalars quite a bit worse. We end up having to sprinkle .get() (Length::get(self) -> T) in many places as we use the length for computations where the result and/or intermediate steps don't have "length"-like semantics, for example the places where we divide by a vector's length, etc. Also it would break a lot of downstream code.

A subsequent commit implements Deref for Length in an attempt to make the sprinkling of get() a bit less ugly, but the cognitive load stays the same everywhere we need the scalars. I postulate that the most common case is to need the scalar directly and therefore getting lengths by default and having to unpack it in a lot of places is a wrong approach.

So as far as T vs Length is concerned I want two sets of methods wherever it makes sense, so that the scalar use case remains the simplest to work with, while still letting people opt into using Length wherever it is valuable to them.

A third option is to make the API as nice to use as possible at the expense of consistency, and keep the nicest names wherever we can. We would skip the prefix for getters where the value can be accessed directly as a scalar member (for example size.width() returns a Length), while methods that compute a scalar would have an equivalent with a prefix (for example vector.length() would continue returning a scalar and we'd have vector.get_length() or other name returning a Length).

kvark commented 4 years ago

Ok, let's defer this until after the release, since it's not breaking.

bors-servo commented 3 years ago

:umbrella: The latest upstream changes (presumably #476) made this pull request unmergeable. Please resolve the merge conflicts.

ogoffart commented 3 years ago

Any update? I was trying to do some type safe unit handling, and this would be quite welcome.

Edit: but actually it would also be nice to have it in more even types like all the accessors of Rect that currently return T directly.

nical commented 3 years ago

I'm still keen on adding strongly typed variants of methods that return scalars where it makes sense using the get_* prefix. @kvark are you on board with it?

kvark commented 3 years ago

Sure

mrobinson commented 11 months ago

We're interested in this feature in Servo, so if there is no opposition, we'd really like to land this. We're happy to continue the discussion about the name of these APIs, but otherwise these seem really useful.