groovy-core icon indicating copy to clipboard operation
groovy-core copied to clipboard

first pr of Groovy-Optional

Open uehaj opened this issue 10 years ago • 13 comments

Last year I proposed a way of transparent handling of java.util.Optional.

http://groovy.329449.n5.nabble.com/Groovier-way-of-handling-Java8-s-java-util-Optional-td5717544.html#a5717561 https://github.com/uehaj/groovy-optional

I think this is very groovy way to handle Optional value, although Optional itself might not be good idea compared to safe navigation, anyway. I imagine you groovy development team are tackling Java8 aware GDK methods?

So I rewrite above proof of concept code to actual PR to current Groovy implemented as a GDK method java.util.Optional.methodMissing().

I hope reviewing this, and please advice me about anything to do. (for example where is the right branch to rebase?) Probably it is needed to add custom type checking rule to avoid STC errors which comes from use of Optional.methodMissing.

Best regards,

uehaj avatar May 18 '14 14:05 uehaj

The dsadvantage of using missingMehod is of course, that any method defined on Optional cannot be called that way. I am worried about map, flatMap, filter, toString, equals

blackdrag avatar May 19 '14 07:05 blackdrag

Thanks your comment. methodMissing captures only call to method which is not exists on Optional, So those methods are called exactly with the same way of Java and it works expectedly. I added test cases:

    void testMap() {
        expect:
        Optional.of("abc").map{it.toUpperCase()} == Optional.of("ABC")
    }

    void testFlatMap() {
        def divide = {it == 0 ? Optional.empty() : Optional.of(3.0/it) }
        expect:
        Optional.of(3.0).flatMap(divide) != Optional.of(1.0) // equals i not dispatched to target.
        Optional.of(3.0).flatMap(divide).get() == Optional.of(1.0).get()
        Optional.of(0).flatMap(divide) == Optional.empty()
    }

    void testFilter() {
        expect:
        Optional.of(3).filter { it % 2 == 0 } == Optional.empty()
        Optional.of(3).filter { it % 2 == 1 } == Optional.of(3)
    }

those behavior is exactly same as Java so upper compatibility is kept perfectly.

But in the case of toString() /equals() / hashCode() might be regarded as a problem. java.util.Optional is of course Java's API class, so in the GDK, it is impossible or not recommended to change the semantics of Java API for compatibility reason isn't it?

So I think we have no choice to override the meaning of Optional's methods -- equals() and toString() and hashCode() -- because we expect the converted Groovy code from Java has same (upper compatible) meaning of java code.

uehaj avatar May 19 '14 13:05 uehaj

what I did mean was more something like this:

[1l,2l].stream().filter {it>1} 

will return a Stream of Long with 2 as single element,

Optional.of([1l,2l].stream()).filter {it>1}

will not work at all, because it would compare the number 1 with a Stream of Longs. It looks a bit constructed with a Stream, but it is to be expected that there are many more tpyes that support that in the future.

What I am saying is that wrapping a value in an Optional and then have the wrapper act transparently lke the object inside is not going to work. And even if we fix hashcode and equals for us, the moment you place it in a collection all bets are off again.

I won't say this pull request is a bad idea, not at all! But first we should clarify what the target of this is and the see what we need to get there

blackdrag avatar May 19 '14 14:05 blackdrag

I get the point. I'd like to try to describe the purpose and target use cases.

Optional is a idea comes from Scala or some FP area(haskell, ocaml,..) and those languages have pattern matching language construct which ordinary used as a simple way to extract Optional/Option/Maybe value but Java and Groovy don't have pattern match now.

The problem of Optional is, to get the value from Optional, You have to do isPresent() check instead of null check.

name = Optional.of("who") or Optional.empty()
if(name.isPresent()){
  System.out.println( name.get() );
}

if you have 2 values, and wrap up to Optional after operation, the code would be like:

Optional<String> fullName = (lastName.isPresent() && firstName.isPresent()) ? 
        Optional.of(lastName.get() + " " + firstName.get())) : 
        Optional.empty();

This is almost as bad as null handling. so we have flatMap.

Optional<String> fullName = 
  lastName.flatMap(ln -> 
  firstName.flatMap(fn -> 
    Optional.of(ln + " " + fn))));

but this still verbose, Especially in Groovy, because we have safe navigation already. with safe navigation and null, you can write:

println(fullName?.plus(' ')?.plus(lastName))

In Scala, for-comprehension (monad comprehension) make more simple, but Groovy don't have now.

To sum up, there is no good enough language support in Java to extract Optional value and operate it and wrap-up again after operation.

By this proposal we can write simple-and intuitive way.

Optional<String> fullName = lastName + firstName

But this works just under restriction which your pointed.

As another idea, how about expand the meaning of "?. " on Optional?

  • [Optional Value] ?. property means [Optional Value].isPresant() ? [Optional Value].get().property : Optional.empty()
  • [Optional Value] ?. method(..) means [Optional Value].isPresant() ? [Optional Value].get().method(..) : Optional.empty()

uehaj avatar May 19 '14 20:05 uehaj

sorry, for the really long delay... using "?." is interesting, but your original problem was

Optional<String> fullName = (lastName.isPresent() && firstName.isPresent()) ? 
    Optional.of(lastName.get() + " " + firstName.get())) : 
    Optional.empty();

and there it won't help.

blackdrag avatar Dec 22 '14 14:12 blackdrag

thank you comment again. You are right. That was wrong. meaning of Optional-aware "?." is not so trivial.

I think ?. is converted depends on weather the type of method parameter is Optional or not. When under Static Type Check, you can generate like following:

// lastName?.plus(" ")
Optional<String> tmp =
    lastName.flatMap { s1 -> Optional.of(s1.plus(" ")) }
assert tmp.get() =="lastname "

// lastName?.plus(" ")?.plus(firstName)
// converted to(under STC):
Optional<String> fullName1 =
    lastName.
    flatMap { s1 -> Optional.of(s1.plus(" ")) }.
    flatMap { s2 -> firstName.flatMap{s3->Optional.of(s2.plus(s3))}  }
assert fullName1.get()=="lastname firstName"

without STC, dynamic type checking with instanceof call is required. for example,

// lastName?.plus(" ")?.plus(firstName)
// converted to(without STC):
Optional<String> fullName3 =
           ((lastName instanceof Optional) ? lastName: Optional.of(lastName)).
           flatMap { s1 -> def arg = " "
               if (arg instanceof Optional) { return arg.flatMap{ s2 -> Optional.of(s1.plus(s2)) } }
               else { return Optional.of(s1.plus(arg)) } }.
           flatMap { s1 -> def arg = firstName
               if (arg instanceof Optional) { return arg.flatMap{ s2 -> Optional.of(s1.plus(s2)) } }
               else { return Optional.of(s1.plus(arg)) } }
assert fullName3.get()=="lastname firstName"

In addition, null check should be done in actual code generation.

uehaj avatar Dec 25 '14 14:12 uehaj

but you see that you cannot write lastName + " " + firstName to get this result? It is because there is no way to express a "?." for operators. It is already the same for safe null navigation, but you while you use null-safe navigation only in a few places normally, you are forced to use optionals all the time, once you start using them. In that optionals are a very intrusive element.

That starts for me the question if a syntax element you have to use all the time for this is the right thing. Instead maybe it should go deeper and become part of Groovy's internals. This idea goes a lot more in the direction of your initial idea. But what I have in mind is much more complex. Instead of having some kind of helper type to do dynamic dispatch with and being lost in static compilation, How about making optional a "wrapper type". That means if the runtime detects an optional, it reacts internally different to it having a value present or not. Then a+b could be transparently done with optionals.

To give an example of what I am thinking here... if you make a method call on receiver, then the runtime will check if the receiver is null, and act different according to that. Of course Optional would be done slightly different... and potentially a lot of places have to change for this...but this is where I see the maximum use...

Do you agree?

blackdrag avatar Dec 25 '14 23:12 blackdrag

I commented on this and then accidentally deleted it, so I will try again.

@uehaj You want to transform a sequence of safe navigation to a sequence of flatMap calls, with the last call being a map. This will remove the need for Optional.of. Your example of:

lastName?.plus(" ")?.plus(firstName)

assumes lastName is optional, but firstName is mandatory. Let's assume lastName is also optional and the space is mandatory. The code is then:

lastName.flatMap { ln -> firstName.map { fn -> fn + " " + ln } }

or if " " is optional then:

lastName.flatMap { fn -> space.flatMap -> { s -> lastName.map { ln -> fn + s + ln } } }

I wrote a dynamically typed method to do this sequence of calls using missingMethod calls so you can write:

foreach {
  fn << firstName
  s << space
  ln << lastName
  yield { fn + s + ln }
}

I intended to rewrite using a macro, but have not gotten around to it. With a macro, this could be typechecked statically. Without a macro (for static typing) you need to ensure the values are monads (so they have flatMap and map) or can be transformed to monads. Scala and Haskell do this using implicits and typeclasses. @paulk-asert has made progress towards implicit conversion for assignment statements. It would be good if this could be extended to general expressions.

mperry avatar Dec 26 '14 08:12 mperry

Hi brackdrag, First of all, I believe my first proposal can be implemented without dynamic method dispatch, under STC , by using custom type checker(type checking extension) which handles methodNotFound event. Of course standard type checker can be changed to do it. So we don't have to lose static compilation. But this is not the point.

I guess your "wrapper type" of Optional handling is done under the similar way where primitive types handling in Groovy isn't it? Java's primitive types are converted to Boxed type versa-visa at run time. If this guess is right, It might works well. Because Optional is Immutable type, referencing target cant be changed throughout lifetime of Optional instance. Probably equals() and hashCode() is forwarded to the wrapped target of Optional? wow, it's nice idea. I agreed.

But it could be hard work. Not only around Java method call, you have to wrap/retrieve operation at all of the retrieve/store operation for the containers which have Optional value, like List<Optional>() or array of Optional, .etc. thx

uehaj avatar Dec 26 '14 13:12 uehaj

Hi, mperry, thanks for comment. I also implemented "type class in groovy" aka Groovyz which introduce implicit parameter based type class. I wrote an article about it (but in Japanese, sorry)

http://uehaj.hatenablog.com/entry/2014/12/24/030136 Learning Type Class thought implementing it in Groovy POC source code: https://github.com/uehaj/groovyz

best regards,

uehaj avatar Dec 26 '14 14:12 uehaj

@uehaj I found the auto translation very hard to read. I would be interested in your explanation of the custom type checker.

mperry avatar Dec 26 '14 15:12 mperry

Hi @mperry I wrote document: https://github.com/uehaj/groovyz if you prefer

uehaj avatar Dec 28 '14 15:12 uehaj

FYI: Rust people are considering to adopt null safe looking operation (?.) on IoResult (corresponding to Optional of java8).

https://github.com/rust-lang/rfcs/blob/master/text/0517-io-os-reform.md#the-error-chaining-pattern The error chaining pattern: File::open(some_path)?.read_to_end()

uehaj avatar Feb 22 '15 08:02 uehaj