patternrubyrailsMinor
Refactor .each where each is redirected into a hash
Viewed 0 times
redirectedeachintowherehashrefactor
Problem
What would be the best way to refactor this :
And this busy statement is the SECOND of two pushes into
I think that I can use map here something like :
Because I don't want to overwrite
But what I'm really searching for is a way to store the entire 'each' into a lambda and say something like :
And move all the hash creation into the
I'm still grasping lambdas and procs
@busy.each do |b|
title = b.title
@events b.id,
:title => title,
:start => b.busy_start_time(b.start_date,b.start_time),
:end => b.busy_end_time(b.end_date,b.end_time),
:allDay => false,
:recurring => false,
:color => 'black',
}@events is initialized as an empty array:@events = []And this busy statement is the SECOND of two pushes into
@events.I think that I can use map here something like :
@events = @events + @busy.map do |b|
stuff
endBecause I don't want to overwrite
@events, I want to add stuff into it.But what I'm really searching for is a way to store the entire 'each' into a lambda and say something like :
@events += @busy(&:create_busy_hash)And move all the hash creation into the
Busy class as create_busy_hashI'm still grasping lambdas and procs
Solution
I think this is where you are trying to get to:
Some additional notes:
class Busy
def event_info
{
:id => id,
:title => title,
:start => busy_start_time(start_date, start_time),
:end => busy_end_time(end_date, end_time),
:allDay => false,
:recurring => false,
:color => 'black',
}
end
end
@events = @busy_items.map(&:event_info)Some additional notes:
- There is no need to initialize
@eventsto an empty array, favor expressions over statements with side-effects (map,select,reduceand so on, instead ofeach).
- I renamed the variable to
busy_itemsto emphasize it's a collection.
Busydoes not sound right for a class name. Classes are usually nouns, not adjectives. What's the nature of this class?
Code Snippets
class Busy
def event_info
{
:id => id,
:title => title,
:start => busy_start_time(start_date, start_time),
:end => busy_end_time(end_date, end_time),
:allDay => false,
:recurring => false,
:color => 'black',
}
end
end
@events = @busy_items.map(&:event_info)Context
StackExchange Code Review Q#51939, answer score: 4
Revisions (0)
No revisions yet.