Feature: Add default result to try method

[dennisjbell]

Rational:
To facilitate chaining of method calls without successive try
wrappers, the addition of a default value to the try call that is
returned when the receiver is nil allows the returned type to be made
homogeneous.

my_string_or_nil.try(&.size)    # => could be Nil or Int32
my_string_or_nil.try(0, &.size) # => will always be Int32

While this is similar to (my_string_or_nil.try(&.size) || 0), the
use of a default in try more elegantly solves the case when the result
could be false or nil.

[bcardiff]

I like the addition. It is aligned with Hash#fetch(key, default) actually.

1. (mandatory) run the formatter, otherwise travis will complain.

2. (opinion) I would leave a single Nil#try method with the same default as Object. The default would be duplicated but it is easier to follow I think. WDYT?

3. (opinion) rename if_nil_value to default so it is the same as Hash#fetch.

[bcardiff]

{|o| o} -> &.itself

[dennisjbell]

yes, I did that idiomatic crystal in other examples, but wanted to show that this way resulted in the same behaviour

[bcardiff]

👍

[dennisjbell]

Ironically, it was originally named default, but I wanted to indicate that it was only used if the value was nil. Will change it back to default, as well as the other items you identified.

[dennisjbell]

@bcardiff Do you want me to flatten the PR to one commit, or keep each commit separate?

[asterite]

exp.try(&.method) || other is actually better because other will be lazily evaluated. I personally don't feel the need to add a default value as an argument. For the case of a boolean, with try you are already getting a truthy/falsey value, which is what you ultimately care about with booleans.

[asterite]

(also the above is a strong idiom in Crystal, the usage of ||)

[matiasgarciaisaia]

@asterite wouldn't that misbehave if false or nil are valid .method return values?

[bcardiff]

An alternative would be to have something as Object/Nil#unnil(&block) so
my_string_or_nil.try(&.size).unnil { 0 } to allow laziness for the default value.

But I think the proposed change to Object/Nil#try is consistent with Hash#fetch.

[kostya]

problem with || that need to add () when chains methods:
asdf.(exp.try(&.method) || other).asdf

seems better:
asdf.exp.try(&.method).else { other }.asdf

[RX14]

@kostya If you have to do that, it's a sign that you should split the expression into multiple variables.

[drosehn]

As a newbie to crystal, I'm wondering why this option would be done as:

my_string_or_nil.try(0, &.size) # => will always be Int32

instead of:

my_string_or_nil.try(&.size, 0) # => will always be Int32

(I have no opinion about which would be better, I'm just wondering which one is "more crystal-ish")

[asterite]

@drosehn The block argument must always come last in the call

[drosehn]

Ah, yes, okay. I understand it better now.

[drosehn]

Let me use a different example, to see if I understand @asterite 's earlier point. Given:

my_craZclass_or_nil.try(CrazyClass.new, &.size).craZ_meth

this statement will always create a new CrazyClass object, even when my_craZclass_or_nil is not nil.

[asterite]

@drosehn Exactly!

[dennisjbell]

@drosehn, The point @asterite makes is valid, though your example makes incorrect use of the proposed try change: the default value is an output value of the method if the receiver is nil, not an input value to the block if the receiver is nil. In your example, the default would likely be -1 or 0, not a computed value.

The intent for this addition is in the spirit of the 80/20 rule where the default is already pre-computed or a simple literal and better handles the case where the receiver might be falsy or the output of the block is falsy. As @bcardiff points out, this has a similar benefit and limitation to using the default on Hash#fetch.

For where the default would need to be compute or memory intensive, then it is recommended to use the ... || compute_the_alternative method, or get the result and wrap it in an if result.nil?.

[drosehn]

Oops, my mistake. I missed one change when I copied the original example to make my new one. What I meant was something more like:

my_craZclass_or_nil.try(CrazyClass.new, &.craZ_dup).craZ_meth

I'm not saying I have any good reason to type that specific statement, but right now I'm too tired to think of a more sensible example. I liked the idea when looking at the example of my_string_or_nil.try(0, &.size), but so far I'm running into "or_nil" situations where I need to create a new object of my own crazy classes, so I'm not sure this would be as useful as I first expected it to be.

Disclaimer: I haven't written all that much crystal, so I know I don't have enough experience to rate the feature. I'm asking questions here just to make sure I understand what people are saying. (and I'm typing in this reply just to make sure the fixed version of my example does make sense!)

[miketheman]

#codetriage

Reading through, it seems like there's a couple of opinions expressed here.

@dennisjbell wants the output of .try to be able to be chained to the next method, echoed by @kostya , and this is performed by a default (the change herein). @bcardiff has requested some changes, that appear to be completed.
There's also a bit of merge conflict that would need to be sorted out.

@asterite and @RX14 are in support of using existing methods such as (my_string_or_nil.try(&.size) || 0) is the current best way to accomplish this goal.

How do we drive this discussion to a conclusion? Should .try have a default result?

[RX14]

In my biased opinion, this issue has been discussed to completion and can be closed.

[ysbaddaden]

Closing. Above discussions made it clear that .try(&block) || default is a better solution.