Create a gist now

Instantly share code, notes, and snippets.

What would you like to do?
module Report
class GenerateWeekly
WeeklyReport = Struct.new(
:week_number,
:date_from,
:date_to,
:count_entries,
:avg_distance,
:avg_speed
)
def initialize(user)
@user = user
end
def call
@user.entries.group_by(&:week).map do |week, entries|
WeeklyReport.new(
week,
weeks_to_date_from(week),
weeks_to_date_to(week),
entries.count,
avg_distance(entries),
avg_speed(entries)
)
end
end
private
def weeks_to_date_from(week)
(Date.new + week.to_i.weeks).to_s.split(',')[0]
end
def weeks_to_date_to(week)
(Date.new + week.to_i.weeks + 7.days).to_s.split(',')[0]
end
def avg_distance(entries)
distances = entries.inject(0){|sum, n| sum + n.distance }
(distances / entries.count).round(2)
end
def avg_speed(entries)
speeds = entries.inject(0){|sum, n| sum + n.speed }
(speeds / entries.count).round(2)
end
end
end
dclausen commented Jan 2, 2017

Since user is referred to once to get the list of entries, why not pass in only what's necessary (in this case, user.entries) as an argument to #initialize?

While there is no absolute true, I find passing the current user more inclined to SRP. This way, your service has the responsibility of building the report to the passed user and not only parse the data. This has it's advantages since the controller does not need to know which data the service will use to build the report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment