find should fail when value is not present

[flbulgarelli]

Returning null is not inherently bad, but leads student into null checking, which is a practice we should avoid.

I propose to simply fail on this scenario, or add a variant with an extra continuation for the fail scenario, or return a maybe functor.

[matifreyre]

+1 A sort of ElementNotFound exception should be raised for these cases.

[PalumboN]

I agree with the 3 options:

• Throw an exception,
• Can choice what to do in fail scenarios,
• Return a Maybe functor.

Maybe they can be different methods.

[npasserini]

I am not sure about Maybe, returning a Maybe when you are sure forces also checkings that from my point of view are not better that null checkings. Also is frequent to see people which use unsafe constructions like Scala's someCollection.find(...).get() to get rid of the maybe.

Throwing an exception could work, providing a callback could work also... but, what about a simple default value?

This is, instead of col.find(condition, [| value]) you could do col.find(condition, value) Just an idea, I see that the former is more powerful, still the latter is simpler to see.

In any case, I would like to think about what to do to avoid constructions like col.find(condition, [|nil]), which you can find everywhere in typical smalltalk code.

[npasserini]

I think that we should have practical examples of using these methods.

[flbulgarelli]

I like the default value too. That could also work.

You are right in that something like a Maybe is mostly interesting if you have a map/flatMap operation, which is like decoupling the detect: and ifNone: parts.

[matifreyre]

Isn't it possible to have them both? I mean, the closure OR the default value. I don't know if is too complex. I know Pharo Smalltalk uses blocks and symbols indistinctly for many collection messages, but I don't know how that is resolved internally. My idea is to achieve something similar: return the object (this) for anything that is not a closure, and evaluate it if it is.

[flbulgarelli]

And what about having three messages?

• find: it just finds the elements, if exists. Otherwise, throws an exception
• findOrElse: if not exists, evaluates the block
• findOrDefault: if not exits, returns the default value

[matifreyre]

I like it, but I still feel that find should just fail.

[flbulgarelli]

Haha, sorry, I meant 'throws an exception'. Yes, It should fail :smile:

[javierfernandes]

Currently there's no "instanceOf" or any way to match/fork execution based on the type of the object, so there's no way to implement a single method for different types of parameters. Besides I guess it could be error prone for the student (?).

Anyway, :+1: to the three methods and avoid the maybe (for the moment :P)

[npasserini]

Unless you decide that you make any object "valuable" i.e. polymorphic with a block, i.e. in any place you expect a no-args block you could put any object instead.

On Tue, Dec 29, 2015 at 10:09 AM, javierfernandes notifications@github.com wrote:

Currently there's no "instanceOf" or any way to match/fork execution based on the type of the object, so there's no way to implement a single method for different types of parameters. Besides I guess it could be error prone for the student (?).

Anyway, [image: :+1:] to the three methods and avoid the maybe (for the moment :P)

— Reply to this email directly or view it on GitHub https://github.com/uqbar-project/wollok/issues/522#issuecomment-167784596 .

[matifreyre]

My thoughts exactly.

On Tue, Dec 29, 2015 at 10:20 AM, Nico Passerini notifications@github.com wrote:

Unless you decide that you make any object "valuable" i.e. polymorphic with a block, i.e. in any place you expect a no-args block you could put any object instead.

On Tue, Dec 29, 2015 at 10:09 AM, javierfernandes < notifications@github.com> wrote:

Currently there's no "instanceOf" or any way to match/fork execution based on the type of the object, so there's no way to implement a single method for different types of parameters. Besides I guess it could be error prone for the student (?).

Anyway, [image: :+1:] to the three methods and avoid the maybe (for the moment :P)

— Reply to this email directly or view it on GitHub < https://github.com/uqbar-project/wollok/issues/522#issuecomment-167784596>

.

— Reply to this email directly or view it on GitHub https://github.com/uqbar-project/wollok/issues/522#issuecomment-167786809 .

[flbulgarelli]

That is two steps away from introducing optional lazy evaluation: if you want a variable to defer its evaluation, wrap into a block.

I like it.

[flbulgarelli]

So, with that change, we have:

• find: it just finds the elements, if exists. Otherwise, throws an exception
• findOrElse: if not exists, evaluates the second argument

[flbulgarelli]

PS: with that semantics, It is clear to me than eval(uate) its a better name that apply. It doesn't have sense to apply a number, but to evaluate it.

[npasserini]

Yet I am not sure about that because (from seeing Pharo/Smalltalk code, for example) detect:ifAbsent: is frequently not enough and thus you see lots of detect: [...] ifAbsent: nil, followed by ifNil. I think that the most useful method would be something like find:ifPresent:ifAbsent, i.e. with two continuations. But then it is too complicated from our students.

I expect that you agree that:

x = col.find(bleh, null) if (x != null) { foo } else { bar }

is even worse than

x = col.find(bleh) if (x != null) {foo} else {bar}

maybe our real solution is try { x = col.find(bleh) foo } catch XXXNotFound { bar }

still, if from all that we should produce a value, we end up in a situation very common in Java

var y try { x = col.find(bleh) y = foo(x) } catch XXXNotFound { y = bar }

maybe we could have try catch be an expression? y = try { x = col.find(bleh) foo(x) } catch XXXNotFound { bar }

I do not know if this would allow also to put returns in the try catch, but I think that if it works for if it could work for try/catch also.

Anyway, you could always force to use an aux method

method computeY( try { x = col.find(bleh) return foo(x) } catch XXXNotFound { return bar } }

y = this.computeY()

Well, I am not sure. But I see that try/catch naturally provides the two continuations of detect:ifPresent:ifAbsent: and therefore is more powerfull that findOrElse().

On Tue, Dec 29, 2015 at 10:31 AM, Franco Leonardo Bulgarelli < notifications@github.com> wrote:

So, with that change, we have:

• find: it just finds the elements, if exists. Otherwise, throws an exception
• findOrElse: if not exists, evaluates the second argument

— Reply to this email directly or view it on GitHub https://github.com/uqbar-project/wollok/issues/522#issuecomment-167789949 .

[javierfernandes]

Seems like a typical "dynamic language" feature. I'm not sure about it, how will it affect the type system inference ?

Using it with a value:

val (Collection<Bird>) birds = ....
val (Bird) aBird = ...

val (Bird) bird = birds.find([Bird b | b.energy > 10], aBird)

But then using it with a closure could mean to "return something"

val (Collection<Bird>) birds = ....
val (Bird) aBird = ...

val (Bird) bird = birds.find([Bird b | b.energy > 10], [ new Bird(...) ])

Or could be to "do something" (sorry not a really good example)

val (Collection<Users>) user = ....
val (User) bird = birds.find([User u | u.name > name], [  app.showLoginError()  ])

I'm starting to like the "Maybe" solution :S

I guess that the typing inference rule would be something like

t(Collection<E>.find(closure, ifNot)) =  commonType(E, type(ifNot.evaluate))

There are at least 2 tricks/complexities with it

1. the "ifNot" could be a no-return block for the case you want to "do something" instead of "returning something"
2. we need to somehow infer the "evaluate()" type.

Anyway, there's also another issue for lazy parameters #374 Since the "&&" message requires this for implementing shortcircuit. It's currently "hardcoded" in the interpreter code. Actually the native impl of the and/or methods receives a java closure.

[javierfernandes]

try-catch is already an expression.

You can write it like this

y = try 
        foo(col.find(bleh))
    catch XXXNotFound
        bar

[flbulgarelli]

The problem with try catch is that it forces the student to use a construction that usually is introduced very late in the didactic sequence.

And it also breaks the 'use exceptions for error handling only' paradigm.

I am not really completely with that paradigm - for example, it is considered a good practice in erlang's code, although it is true that they distinguish between errors and non-local-returns - but for didactic purpouses it introduces an extra complexity we should avoid.

[flbulgarelli]

BTW: the reasons of using exception for error handling only, as far as I understand, it is very related with the fact that in JVM/C++/CLR derived languages, they tend to be much less efficient than a simple if, and not really a conceptual problem. I understand that it is not the case of Smalltak

Some could argue that they are more difficult to read, but I am not sure about that.

[javierfernandes]

But "error handling" is a context dependent / subjective term. For the collection point of view not matching any value for a find seems like "an error" so it throws exception. Maybe for the caller too, then it won't handle it and propagate it (still error meaning) But maybe my calling logic has certain logic to recover from that "collection-level" error and do something with a try catch.

An exception can start in a given piece of code as an "error" but then become just flow control logic. What we are actually trying to avoid here is "using return value for modelling errors situations".

I agree that introducing try catch early is a problem. But I guess that it is also extra-complexity or baggage for the student to know that a method behaves differently depending on the type of the argument.

I still think that the best solution is: find, findOrElse, findOrDefault (?). It's more explicit

[flbulgarelli]

Ok, you are right.

You cited the type system issue, perhaps that is the biggest problem.

So I really don't know now :stuck_out_tongue: Both options seem ok to me, so I vote for implementing the simpler one.

[flbulgarelli]

Requires #567 to test it

[fdodino]

I think this is already done.