RFE: add missing methods to Optional, OptionalInt, OptionalLong and OptionalDouble
I noticed that, while Stream and its primitive equivalents have multiple map and flapMap methods, Optional and its primitive equivalents were missing those. Since these can still be very useful, I wrote a patch to add the following methods: * Optional: mapToInt, mapToLong, mapToDouble, flatMapToInt, flatMapToLong, flatMapToDouble * OptionalInt: map, mapToObj, mapToLong, mapToDouble, flatMap * OptionalLong: map, mapToObj, mapToInt, mapToDouble, flatMap * OptionalDouble: map, mapToObj, mapToInt, mapToLong, flatMap I did not add any more flatMap methods to OptionalInt, OptionalLong and OptionalDouble because these also don't exist on IntStream, LongStream and DoubleStream. In addition, I also added the missing or method to OptionalInt, OptionalLong and OptionalDouble based on that of Optional. My OCA has been processed on 2019-01-22. Rob
This is a very nice patch, complete with spec and tests, and evidence of OCA. But, before we talk about patches and code, let's step back and talk about stewardship. With API proposals, we like to start with problems, rather than solutions: what problems we are concerned with, and are these are problems that need to be solved in the JDK? Assuming we like the answers to the former, we can then move on to whether the proposed solution is the right one, and then to details like spec and code review. We are currently trying to avoid adding anything to OptionalInt/Long/Double, because our current plan is to replace these with a specialized Optional<int> once Valhalla delivers. Any methods that differ from the base type (Optional) creates potential roadblocks to such a migration, so we'd prefer to leaves these classes be. To the main question, though, whether Optional should have specialized map and flatMap methods ... I'm skeptical. The goal was never to create parity between Optional and Stream, and my gut reaction is that the return on additional API surface area here is negative. But, we can surely have a discussion on this. On 2/16/2019 7:52 AM, Rob Spoor wrote:
I noticed that, while Stream and its primitive equivalents have multiple map and flapMap methods, Optional and its primitive equivalents were missing those. Since these can still be very useful, I wrote a patch to add the following methods:
* Optional: mapToInt, mapToLong, mapToDouble, flatMapToInt, flatMapToLong, flatMapToDouble * OptionalInt: map, mapToObj, mapToLong, mapToDouble, flatMap * OptionalLong: map, mapToObj, mapToInt, mapToDouble, flatMap * OptionalDouble: map, mapToObj, mapToInt, mapToLong, flatMap
I did not add any more flatMap methods to OptionalInt, OptionalLong and OptionalDouble because these also don't exist on IntStream, LongStream and DoubleStream.
In addition, I also added the missing or method to OptionalInt, OptionalLong and OptionalDouble based on that of Optional.
My OCA has been processed on 2019-01-22.
Rob
I was actually missing at least one of the map methods. I have a method that is returning an OptionalInt, and I wanted to easily turn that into an Optional<Integer> by applying mapToObj(Integer::valueOf). The same could also be done for OptionalLong -> Optional<Long> and OptionalDouble -> Optional<Double>. However, Valhalla's Optional<int> will indeed make OptionalInt more or less superfluous (likewise for Optional<long> and Optional<double>). That raises one question - would it be worth adding these "missing" methods for just a few releases, after which they would no longer be necessary? They would not differ from the base type any more than IntStream, LongStream and DoubleStream differ from Stream, so that shouldn't block it. Speaking of the OptionalInt returning method, that is a version of Integer.parseInt that does not throw an exception but instead returns an OptionalInt (to prevent having the exception created and caught, only to be ignored). In line with C# I was calling it tryParseInt. I was actually intending to have an RFE for that one too, but in light of the upcoming Optional<int> I think it's probably a better idea to wait with that until it's possible to return an Optional<int>. Otherwise this method would be stuck with the "old" OptionalInt return type. On 19/02/2019 17:01, Brian Goetz wrote:
This is a very nice patch, complete with spec and tests, and evidence of OCA.
But, before we talk about patches and code, let's step back and talk about stewardship. With API proposals, we like to start with problems, rather than solutions: what problems we are concerned with, and are these are problems that need to be solved in the JDK? Assuming we like the answers to the former, we can then move on to whether the proposed solution is the right one, and then to details like spec and code review.
We are currently trying to avoid adding anything to OptionalInt/Long/Double, because our current plan is to replace these with a specialized Optional<int> once Valhalla delivers. Any methods that differ from the base type (Optional) creates potential roadblocks to such a migration, so we'd prefer to leaves these classes be.
To the main question, though, whether Optional should have specialized map and flatMap methods ... I'm skeptical. The goal was never to create parity between Optional and Stream, and my gut reaction is that the return on additional API surface area here is negative. But, we can surely have a discussion on this.
On 2/16/2019 7:52 AM, Rob Spoor wrote:
I noticed that, while Stream and its primitive equivalents have multiple map and flapMap methods, Optional and its primitive equivalents were missing those. Since these can still be very useful, I wrote a patch to add the following methods:
* Optional: mapToInt, mapToLong, mapToDouble, flatMapToInt, flatMapToLong, flatMapToDouble * OptionalInt: map, mapToObj, mapToLong, mapToDouble, flatMap * OptionalLong: map, mapToObj, mapToInt, mapToDouble, flatMap * OptionalDouble: map, mapToObj, mapToInt, mapToLong, flatMap
I did not add any more flatMap methods to OptionalInt, OptionalLong and OptionalDouble because these also don't exist on IntStream, LongStream and DoubleStream.
In addition, I also added the missing or method to OptionalInt, OptionalLong and OptionalDouble based on that of Optional.
My OCA has been processed on 2019-01-22.
Rob
participants (2)
-
Brian Goetz
-
Rob Spoor