2
\$\begingroup\$

It's a task from a Ruby-course, in which I'm currently enrolled: Ruby Course - Page

Precisely it's one of the assignments for the first week.

Following idea: Your are given a list in which finished work-tasks are logged. [ {work: "item 1", date: "2017-04-26", time: 20}, {work: "item 2", date: "2017-04-27", time: 27}, ...

You shall write a function which computes the daily work-time for the different months. Means: The average daily work-time in April, avg. time in May, ... in June. And so on ...

A data-structure, to work upon, was given. Even the result, which is expected for that data-structure: { "2017-04" => 40, "2017-05" => 14 }.

I was able to write a function, which passed all unit-tests. Here it is:

#!/usr/bin/ruby
tasks = [
 {work: "item 1", date: "2017-04-26", time: 20},
 {work: "item 2", date: "2017-04-27", time: 27},
 {work: "item 3", date: "2017-04-27", time: 33},
 {work: "item 4", date: "2017-05-05", time: 20},
 {work: "item 5", date: "2017-05-06", time: 12},
 {work: "item 6", date: "2017-05-14", time: 10},
]
# Expected result : { "2017-04" => 40, "2017-05" => 14 }
def work_per_month(tasks)
 days_aggregate = {}
 tasks.each do | task |
 key = task[:date]
 if days_aggregate.key?(key)
 days_aggregate[key][0] = days_aggregate[key][0] + task[:time]
 else
 arr = []
 arr[0] = task[:time]
 days_aggregate[key] = arr
 end
 end
 months_aggregate = {}
 days_aggregate.each do | key, task |
 parts = key.split("-")
 k = "#{parts[0]}-#{parts[1]}"
 if months_aggregate.key?(k)
 months_aggregate[k][0] = months_aggregate[k][0] + task[0]
 months_aggregate[k][1] = months_aggregate[k][1] + 1
 else
 arr = []
 arr[0] = task[0]
 arr[1] = 1
 months_aggregate[k] = arr
 end
 end
 avg_hours_month = {}
 months_aggregate.each do | key, data |
 avg_hours_month[key] = data[0] / data[1]
 end
 avg_hours_month
end
puts work_per_month(tasks) # Returns {"2017-04"=>40, "2017-05"=>14} 

Please take into account that I started Ruby programming just a week ago.

It works and it has passed the tests. But I'm aware that it is clumsy.

Is there are more elegant way to solve the described task?

Without having this sequence of loops?

asked Jul 21, 2019 at 11:20
\$\endgroup\$
1
  • \$\begingroup\$ The example result seems wrong: the average of (20 + 27 + 33) / 3 is 26.666.., not 40. Now, I don't know about elegant, especially for the first week (!), but tasks.group_by {|h| h[:date][0..6]}.reduce({}) {|out, (k, v)| out[k] = v.reduce(0.0) {|sum, h| sum + h[:time] } / v.size; out} should do it as a one-liner. Not that it should be a one-liner, reason here is just so it fits in a comment box. \$\endgroup\$ Commented Oct 3, 2019 at 14:13

1 Answer 1

1
\$\begingroup\$

Rubocop Report

Rubocop was able to solve minor layout issues regarding the use of white spaces. One note-worthy change it proposed is:

  • [Corrected] Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
days_aggregate.each do | key, task |
 parts = key.split("-")
 parts = key.split('-')

A bigger complexity issue it found was:

  • Metrics/AbcSize: Assignment Branch Condition size for work_per_month is too high. [33/15]
  • Metrics/MethodLength: Method has too many lines. [30/10]

This means your method work_per_month is doing way too much. It has more than double the AbcSize than the suggested threshold [33/15]. You should split this method in sub-routines.

answered Jul 21, 2019 at 11:52
\$\endgroup\$
2
  • \$\begingroup\$ Thanks a lot for your answer. You are of course right. 41 lines is indeed too much. Normally it should be 30 lines maximal. \$\endgroup\$ Commented Jul 21, 2019 at 12:03
  • \$\begingroup\$ The default is set even lower than 30: it's 10 rubocop.readthedocs.io/en/latest/cops_metrics \$\endgroup\$ Commented Jul 21, 2019 at 12:05

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.