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

Units API Rewrite #6778

Closed
wants to merge 2 commits into from
Closed

Conversation

narmstro2020
Copy link
Contributor

@narmstro2020 narmstro2020 commented Jun 27, 2024

So this PR is based on feedback and discussion from PR #6710, #6696, #6684, #6676

I know some of those are mine. I will consider those dead and will leave them only for references to the feedback and notes.

I'm going to start with taking the existing units API and breaking it down into separate measurement type packages, angle, time, dimensionless, angularvelocity, etc.

Each will have an a class equivalent to the old Measure interface and ImmutableMeasure class which will handle immutable measures. (i.e. Angle)

Each will have a class that will handle the units of that type (i.e. AngleUnit)

Each class that handles Immutable measures will have a child class that handles mutable ones (i.e. MutableAngle).

At first (and probably at the end) many of the packages will be near duplicates of each other except for minor variations to handle division, multiplication, etc.

For the most part no generics will be used. They might come back in a limited form as this is further optimized, but no guarantees.

Every unit of a certain measurement type will have all of its derived units go back to the S.I. standard unit. As an example feet will be based on meters and inches will be based on meters. New units will not be based off of other ones unless that other one is the S.I. base (or derived as in the case of m/s). I've found it confusing when seeing the word base in the old API and not knowing if S.I. base or a unit that a new one is based on is used.

Units that are derived from the standard S.I. base units will be done so here as well much like the way AngularVelocity is done.

I'll not be focusing on UnitBuilder at this time. I'll effectively deprecate it. Though custom units of a type can still be created through the constructors and static methods of that type.

Of course feeback is welcome. I'll leave this in draft form until I've effectively replaced all of the existing units with the new setup.

@WispySparks
Copy link
Contributor

WispySparks commented Jun 27, 2024

A rewrite has begun here as well by @SamCarlberg

@narmstro2020
Copy link
Contributor Author

narmstro2020 commented Jun 27, 2024

@WispySparks
Well how about that. I didn't think to check his repos.
It looks he and I are on the same path in our way of thinking.

@SamCarlberg
Sorry I didn't get back with you sooner on this. Looking at your work in progress it looks like we're on the same wavelength.

I guess I'll just leave this dead for now.

@SamCarlberg
One suggestion is to take EnergyUnit and VoltageUnit out of BaseUnits

@SamCarlberg
Copy link
Member

Yes, this is very similar. However, base types will be required to allow classes to be unit-agnostic when necessary:

class ConfigurableThing<U extends Unit> {
  Measure<? extends U> configuredValue;
}

var byDistance = new ConfigurableThing<DistanceUnit>();
byDistance.configuredValue = Feet.of(...);

var byTime = new ConfigurableThing<TimeUnit>();
byTime.configuredValue = Minutes.of(...);

Mutable types inheriting from an immutable base also means you can't get JVM optimizations from final fields, use records, or migrate to value types if project valhalla ever lands. This is why the approach I've been taking uses an interface with mutable and immutable implementations. It also opens up the possibility to codegen (most of) the implementation types.

@narmstro2020
Copy link
Contributor Author

Can't wait to see the finished product.

Sorry in my previous comment I just mentioned Energy and Voltage as being separated from Base types just to match up with SI

@PeterJohnson
Copy link
Member

Superseded by #6958.

@narmstro2020 narmstro2020 deleted the UnitsRedo branch August 29, 2024 20:48
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.

4 participants