Search code examples
rubyenumerator

chaining ruby enumerator functions in a clean way


I just finished a course on ruby where the instructor takes a list of movies, groups them, then calls map, sort, and reverse. It works fine, but I don't find the syntax to be very readable and I'm trying to figure out if what I have in mind is valid. I come from a c# background.

#we can reformat our code to make it shorter
#note that a lot of people don't like calling functions on the
#end of function blocks. (I don't like the look, either)
count_by_month = movies.group_by do |movie|
                        movie.release_date.strftime("%B")
                    end.map do |month, list|
                        [month, list.size]
                    end.sort_by(&:last).reverse

What I am wondering is if I can do something like

#my question: can I do this?
count_by_month = movies.group_by(&:release_date.strftime("%B"))
                   .map(&:first, &:last.size)
                   .sort_by(&:last)
                   .reverse

#based on what I've seen online, I could maybe do something like
count_by_month = movies.groupBy({m -> m.release_date.strftime("%B")})
                   .map{|month, list| [month, list.size]}
                   .sort_by(&:last)
                   .reverse

Solution

  • As a number of people in the comments suggest, this is really a matter of style; that being said, I have to agree with the comments within the code and say that you want to avoid method chaining at the end of a do..end.

    If you're going to split methods by line, use a do..end. {} and do...end are synonymous, as you know, but the braces are more often used (in my experience) for single-line pieces of code, and as 'mu is too short' pointed out, if you're set on using them, you may want to look into lambdas. But I'd stick to do..end in this case.

    A general style rule I was taught that I follow is to split up chains if what is being worked with changes class in a way that might not be intuitive. ex: fizz = "buzz".split.reverse breaks up a string into an array, but it's clear what the code is doing.

    In the example you provided, there's a lot going on that's a bit hard to follow; I like that you wrote out the group_by using hash notation in the last example because it's clear what the group_by is sorting by there and what the output is - I'd put it in a [well named] variable of its own.

    grouped_by_month = movies.groupBy({m -> m.release_date.strftime("%B")})
    count_by_month = grouped_by_month.map{|month, list| [month, list.size]}.sort_by(&:last).reverse
    

    This splits up the code into one line that sets up the grouping hash and another line that manipulates it.

    Again, this is style, so everyone has their own quirks; this is simply how I'd edit this based off a quick glance. You seem to be getting into Ruby quite well overall! Sometimes I just like the look of a chain of methods on one line, even if its against best practices (and I'm doing Project Euler or some other project of my own). I'd suggest looking at large projects on Github (ex: rails) to get a feel for how those far more experienced than myself write clean code. Good luck!