Sdk: (Range)Error problems: difficult to understand: messages

[rakudrama]

RangeError messages are difficult to understand.

Examples:

RangeError (index): Invalid value: Not in range 0..2, inclusive: 3
RangeError (index): Index out of range: index should be less than 2: 2

One can eventually learn to interpret the messages, but the difficult in understanding the messages has caused users to try to create their own messages via interpolation in calls to methods like RangeError.checkValueInInterval. This comes at a high cost since in all programs (except those that intentionally throw these errors), the String construction is wasted.

@peter-ahe-google has also observed difficulty understanding the messages

@lukechurch: Do you know of usability research that tells us how to compose such messages?

---

One of the problems with helper methods like RangeError.checkValueInInterval is that the language does not help one write these helpers without falling into this pitfall of unnecessary computation.
The assert statement has an optional message part that is evaluated only when the test fails. There is not feature in the language that abstracts over this behaviour - you can't write a procedure that has a conditionally evaluated argument (without passing a closure, which is itself expensive.).

[lrhn]

Since I wrote the messages, I (unsurprisingly) don't have any problem reading them, so input is appreciated. If we have a better wording, I'm all for changing the messages.

The messages are created by the RangeError and IndexError classes based on parameter name (index), value (3) and either the required range (0..2) or the length of the thing being indexed (2).

The language does indeed have only one way to avoid computing something if it isn't going to be needed (assert), and likely won't have any other kind of lazy evaluation/call-by-name. That's why the computation is usually handled in the throw statement itself, when we know for sure that it's needed. It's hard to abstract over that, and that is one reason I sometimes use if (test) throw ... inline instead of using the helper methods.

One solution to that could be to use runtime-interpreted templates instead of string messages, so the error message can be computed only when needed.
Something like new RangeError.value(value, "foo", "@param must not be @value, it's bad, m'kay?").
It comes with a cost, obviously, and it won't necessarily make error messages more consistent. It's better to have a good default pattern that everybody uses, instead of making each user responsible for their own error message formatting on top of doing their real programming.

[rakudrama]

Some limited form of runtime-interpolation has several advantages:

• Different parts could have different roles, requiring different printing. e.g. you would quote a variable name differently to a string irritant.
• Values from the program could be safe-printed to avoid errors. No constructed messages do this properly.

[peter-ahe-google]

I think we should change the default message from its current form to:

"The value of '$name' ($value) must be between $min and $max."

[lrhn]

That's definitely doable as the default format if name is provided and message isn't, and the max isn't null.
I'm not absolutely happy about it, but that's a philosophical problem - do you report an error by saying what is wrong or saying what would be right?
I probably tend to saying "$value is not between $min and $max", that is, describing the error, not "$value must be between $min and $max", which describes the rule being violated.

What would you prefer in the cases where name is omitted or message is provided?

Maybe start with

The value ${name != null ? "of '$name' ": ""} (${Error.safeToString(value)) must ...

and continue with:

• min == null : " be less than or equal to $max"
• max == null: " be greater than or equal to $min"
• otherwise: `" be between $min and $max (inclusive)"
  (I might also have a special case for min == 0, max == null, as "must not be negative", and one for min == max).

And then, if there is a message, add that afterwards as " ($message)" (but that's likely to not work well with the current messages that assume they come after a colon).

[peter-ahe-google]

What about:

"The provided value, $value, must be between $min and $max."

[peter-ahe-google]

@lukechurch is it better to use "should"?

[jamesderlin]

Here's an example of someone getting confused: https://stackoverflow.com/questions/53710596/flutter-range-error-invalid-value-inclusive-1

I think the comma in the current RangeError message is confusing. I think even slight changes such as:

RangeError (index): Invalid value: Not in range 0..2 (inclusive): 3
RangeError (index): Invalid value: Not in range [0, 2]: 3

would be clearer.

[Sacchid]

That's definitely doable as the default format if name is provided and message isn't, and the max isn't null.
I'm not absolutely happy about it, but that's a philosophical problem - do you report an error by saying what is wrong or saying what would be right?
I probably tend to saying "$value is not between $min and $max", that is, describing the error, not "$value must be between $min and $max", which describes the rule being violated.

What would you prefer in the cases where name is omitted or message is provided?

Maybe start with

The value ${name != null ? "of '$name' ": ""} (${Error.safeToString(value)) must ...

and continue with:

• min == null : " be less than or equal to $max"
• max == null: " be greater than or equal to $min"
• otherwise: `" be between $min and $max (inclusive)"
  (I might also have a special case for min == 0, max == null, as "must not be negative", and one for min == max).

And then, if there is a message, add that afterwards as " ($message)" (but that's likely to not work well with the current messages that assume they come after a colon).

Can we also display name of variable (optional)?
As when there are multiple variables or parameters it's very difficult to understand which variable caused RangeError

[hasen6]

I think a simpler fix that wouldn't change the error messages completely but would make them a lot clearer would be :

RangeError (index): Not in range 0..6, inclusive. Invalid Value: -1
or equally:
RangeError (index): Invalid Value: -1. Not in range 0..6, inclusive.

Not sure what other people think but how it is it's all a bit back to front, and this seems clearer at least to me. The way it is it seems that inclusive: -1 is referring to -1 since it has a colon right after it.

[jamesderlin]

I have a pending change to improve this slightly: https://dart-review.googlesource.com/c/sdk/+/146024

[lrhn]

@jamesderlin's changes landed. No further plans to change the messages (for now).