`Enumerable#find` and `Enumerable#index` don't have raising variants.

[yxhuvud]

Feature Request

• Is your feature request related to a problem? Please describe clearly and concisely what is it.

Both Enumerable#find or Enumerable#index will return nil if the search for element isn't found. In some cases there is already knowledge that there will be an element that matches the block. In that case the user will be forced to do something like

array.find { .. }.not_nil!
array.index { .. }.not_nil!

or equivalent to erase the nilability of the return type.

• Describe the feature you would like, optionally illustrated by examples, and how it will solve the above problem.

I propose adding

array.find! { .. }
array.index! { .. }

to Enumerable, where if the block doesn't return true it will raise an exception. I'd suggest adding a Enumerable#NotFoundError for these, but am open to other suggestions.

• Describe considered alternative solutions, and the reasons why you have not proposed them as a solution here.

Another common pattern for doing the same thing would be to separate the existing methods into find and find?, index and index? respectively. That would also be fine and match existing patterns for this, but as the methods already exist I'd prefer to not break backwards compatibility.

• Does it break backward compatibility, if yes then what's the migration path?

No

[straight-shoota]

Adding these non-nilable methods sounds good to me 👍

I have some thoughts about the error type. Technically, NotFoundError would be closely related to EmptyError. The latter is a specialization for the case where you are fine with finding just any value but there simply is none. So I'm wondering if it makes sense to introduce a new error type or if we could merge those two. The new name would be NotFoundError, with EmptyError as a deprecated alias. Not sure if that would need to be considered a breaking change. If yes, need to decide if we want to use EmptyError for now in find! and index! or introduce a new type and merge them later.

Enumerable::EmptyError is currently raised from these methods when the enumerable is empty:

• Enumerable#first
• Enumerable#reduce
• Enumerable#max_by
• Enumerable#max_of
• Enumerable#min_by
• Enumerable#min_of
• Enumerable#minmax_by
• Enumerable#minmax_of

[asterite]

I don't think it makes sense to merge the two errors. If you get "NotFoundError" from Enumerable#max_by, it wouldn't look very nice. I can imagine this dialog in someone's brain:

• What do you mean you can't find it?
• How can you possibly not find the maximum of a given set of values?
• Ooooh... it's because... the enumerable was empty!

Seeing Enumerable::EmptyError makes this immediately obvious.

[HertzDevil]

May want to consider Indexable#bsearch!, #bsearch_index!, and #rindex! too. (I thought Indexable#rfind existed too but that's not the case...?)

[yxhuvud]

I have thought about that, and I agree that having those available would follow the general pattern (and possibly make @straight-shoota a bit more happy with regards to exceptions with few usages). On the other hand I have never tended to use any of those often enough that I have felt an actual need for a more streamlined version, and I'm a little skeptical about adding things just for the sake of orthogonality if I havn't noticed any actual need. :shrug:

[straight-shoota]

Thinking about the error types again, I agree now that separate NotFoundError and EmptyError makes sense. But the latter could probably be a sub type of NotFoundError. Same for IndexError and KeyError. They're all special reasons for an element not being found.

• Enumerable::NotFoundError
  • IndexError
  • KeyError
  • Enumerable::EmptyError

[yxhuvud]

IndexError and KeyError are not defined as part of Enumerable though, and are used in a lot of other places. Changing the existing ones to depend on Enumerable would be quite weird (especially for the usages outside Enumerable), and splitting out the usages inside of Enumerable would be a breaking change. Restricting the discussion to only the NotFound and Empty, I see the is-a relationship but have a bit of a problem envisioning a scenario where one would want to catch both, but not Exception.

[straight-shoota]

I'm not sure if IndexError and KeyError namespace matters much. They'd all be defined as part of the core lib, so it's not a strict problem. Just looks weird. But we might consider moving stuff around to make it better.

a bit of a problem envisioning a scenario where one would want to catch both, but not Exception.

Yeah, I see that's a valid point. I don't have a specific use case in mind as well.

In fact, I don't think these exceptions are really intended to be rescued specifically. When you do a lookup that is expected to fail sometimes, you should use the nilable accessor (or fetch) instead of rescuing a potential index or key exception. These exceptions are only raised when the lookup is expected to succeed but didn't for some reason. They bubble up to some kind of generic rescue handler (such as the current fiber's).

I did a quick code search for rescue .*(Key|Index|Empty)Error and found only a handful of uses (none in stdlib). Most of them should be refactored.