Any idea how to refactor this method?

Ruby on Rails
1 
def booked_days                                                              
  days = []       
    
  reservations.each do |reservation|                                                                                     
    ( (reservation.check_in)..(reservation.check_out - 1.day) ).each { |d| days << d }                                   
  end                                                                                                                    
                                                                                                                              
  days                                                                                                                   
end
2

Hi @alexpereira,

First off, any time youā€™re setting up an empty array and then shoving stuff into it from an enumeration, youā€™ve found an excellent candidate for #inject (or perhaps #each_with_object)! I think #inject isnā€™t used enough given how powerful it is.

Iā€™d also consider adding a method to your Reservation class that returns the date range, as this current method is in charge of doing a lot of different things and has to have knowledge of reservations. Maybe each reservation can return its own booked days and then this class you have (which might be for a customer?) can map and flatten all the booked days from the different reservations?

3

Thanks for your help dude, the #each_with_object do the job, and add a method on Reservations, result much better idea.

def booked_days 
	reservations.each_with_object([]) do |reservation, days|
		reservation.booked_days.each { |day| days << day }
	end
end
1 Like
4

Shorter, cleaner methods for the evolutionary win. Already looks better!

5

This is more idiomatically an inject. Also, I think you can get rid of the eachā€¦

def booked_days
  reservations.inject([]) do |days, reservation|
    days += reservations.booked_days
  end
end

Thereā€™s one key difference between inject and each_with_object, but itā€™s not a problem in this case. inject accumulates based on the return value of the block, whereas each_with_object assumes you will mutate the object if you want it changed. I tend to prefer ruby methods over those from active support where possible and it works well here.

2 Likes
6

I spend so little time doing non-Rails coding that I hadnā€™t even realized that #each_with_object is an AS method.