turingschool-examples / ruby-exercises

A collection of exercises to practice various aspects of Ruby
MIT License
237 stars 767 forks source link

max_by enumerable, should be min_by enumerable #84

Closed elyhess closed 4 years ago

elyhess commented 4 years ago

I stumbled on what I believe to be a typo in the fix_using_max_by_test.rb in the enumerables folder. I believe max_by should actually be min_by. After consulting with several other students, we came to the consensus that this could very well have been a typo.

https://github.com/turingschool/ruby-exercises/blob/386e10e94fe3cc018ae5da654977723b3851ed04/enumerables/exercises_1/find_using_max_by_test.rb#L18-L24

In the thread where we discussed this

Megan Campbell said:

I still think it might be a typo. it seems like max_by, min_by go hand in hand and I'm not seeing any other exercises(didn't really look just skimmed) that specifically deal with min_by so to me it looks like this exercise covers both.

Joe Jiang had replied stating:

i did min_by, I see no point in altering a method to fit another when it has a specific function

with Josh Thompson chiming in at the end

Strong possibility that this is a typo.

I implemented this fix in PR #83

BrianZanti commented 4 years ago

Thanks for submitting this @elyhess, as well as the others mentioned who participated in the discussion. In this case, changing this max_by to a min_by is an appropriate solution. However, given the name of the exercise, I believe the intent was that every problem in this set can be solved using max_by. In this case, a solution using max_by could be

    found_word = words.max_by do |word|
      -1 * word.length
    end

By adding in the -1, you are able to sort by shortest length first. While using min_by would be an appropriate solution, I don't think it is the only solution and thus I don't think a permanent change to the exercises is warranted.

Thanks again for the participation!