I'm never against fixing this issue but I have one concern. Currently, Range#max is not consistent with Enumerable#minmax.
p ("a".."aa").max #=> "aa"
p ("a".."aa").minmax #=> ["a", "z"]
Thus, if Range#minmax is simply implemented, it will make it consistent. This is not always good because it means that the fix brings incompatibility.
Do you have any use case? Or, are you facing any concrete issue that is caused by this inconsistency? If so, we can believe that it would be worth fixing this issue. If not, we need to consider.
My use case has to do with Regexp quantification. I can go into more detail, but to describe it quickly, I want to provide information about how many chars a Regexp can match in https://github.com/ammar/regexp_parser. Ranges, some ending with Infinity, are the most natural choice for this, but minmax would be useful in the related code. Also, I don't want to hand out "dangerous" Ranges to gem users. Maybe I will #extend the Ranges with a safe minmax.
I think this is a bug we should fix, even if it breaks code relying on this bug ("all bug fixes are incompatibilities" :)).
I worked on a patch for #15867, before realizing mohsen created a pull request for this. My patch ended up being very similar to mohsen's, it is attached. The main differences are that my patch calls range_min/range_max C functions directly instead of calling min and max using rb_funcall, and mohsen's patch includes tests and specs, and mine only tests.
Does anyone have an opinion on whether minmax should call overridden min and max methods? Or do we expect if you override min or max, you should also override minmax? I don't have a strong opinion either way.
Does anyone have an opinion on whether minmax should call overridden min and max methods? Or do we expect if you override min or max, you should also override minmax? I don't have a strong opinion either way.
Other classes including Enumerable also require minmax to be overridden individually. Set or SortedSet are examples from the stdlib. So I guess it would be more consistent to call the C functions.
Thinking about this a bit more generally, I'm wondering whether Enumerable#minmax should actually use rb_funcall to get min and max.
This would fix the problem described in this issue.
But perhaps more importantly, it eliminates the trap of forgetting to implement #minmax whenever you override #min and/or #max.
Right now, #minmax is effectively broken for every case where #min and/or #max is overridden to optimize performance, to use custom checks, or to return a custom value -- unless #minmax is also overridden with def minmax; [min, max] end.
As Range shows, arguably even ruby core coders have fallen into this trap ...
Range#minmax was previous not implemented, so calling #minmax on
range was actually calling Enumerable#minmax. This is a simple
implementation of #minmax by just calling range_min and range_max.