On Sat, 4 Oct 2025 07:04:03 GMT, Eirik Bjørsnøs <[email protected]> wrote:
> ```
> // Currently Float16 is a value-based class and in future it is
> ```
>
> in _the_ future
>
> ```
> // IEEEremainder / remainder operator remainder
> ```
>
> Would one remainder suffice for this remainder reminder?
>
> ```
> private final short value;
> ```
>
> Consider adding a field comment to align with Float, Double and friends.
>
> ```
> private static final Float16 ZERO = valueOf(0);
> ```
>
> Other private implementation constants in this class have comments. These
> would be rather obvious, but perheps we can consider adding them just for
> consistency?
>
> Should we consider ordering private vs. public constants? Now they seem a bit
> mingled, making it harder for the eye to scan the public API of the class.
>
> Line 372:
>
> ```
> return new Float16((short)(sign_bit | 0x7c00));
> ```
>
> `0x7c00` here is `EXP_BIT_MASK`, right?
>
> ```
> // toHexString, line 288:
> return s.replaceFirst("p-1022$", "p-14");
> ```
>
> Are we okay to invoke regex from such low level code?
I'll push an update to address most of these points.
For the use of regex, is it an aesthetics question or a JDK bootstrapping
question?
For aesthetics, I think it is fine --the code concisely conveys the intention
of the desired transform and is shorter than writing out the needed state
machine by hand. If this code turned out to be high duty cycle and the regex
overhead was a problem, it would be reasonable to re-implement the
functionality more directly.
For bootstrapping that isn't a direct concern here in an incubating type, but
this code was ported over from java.lang.Float so I'll address the comment.
Even it use of regex in the wrapper classes would be a problem in theory, this
particular use doesn't seem to be a problem in practice since the code in Float
has been stable for at least 18 years, and probably a few years longer going to
back to JDK 5 when hex floating-point literals were introduced.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/27625#issuecomment-3369801040
