1
\$\begingroup\$

I'm using Codefights to improve my skills. This challenge is to change the number submitted as total minutes into the digital read out on a clock i.e. hh:mm and then add all the numbers to together. So h + h + m + m.

My code works and I've passed all the tests, but it duplicates the same process for variables I've named number and float. What are some ways I can clean this up? At the bottom of the code I put the tests with the correct answers commented out next to them.

def lateRide(n)
  n = n.to_f
  n = n/60
  float = n % 1
  number = n - float
  number = number.to_i

  float = float * 60
  float = float.round.to_i

  if number > 9
    tens = number/10
    ones = number-(tens*10)
  else
    tens = 0
    ones = number
  end

  if float > 9
    ftens = float/10
    fones = float-(ftens*10)
  else
    ftens = 0
    fones = float
  end
  tens + ones + ftens + fones
end

puts lateRide(240) #4
puts lateRide(808) #14
puts lateRide(1439) #19
puts lateRide(0) #0
puts lateRide(23) #5
puts lateRide(8) #8
\$\endgroup\$

closed as unclear what you're asking by Jamal Mar 24 '17 at 3:06

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 2
    \$\begingroup\$ Please change the title to a brief summary of the purpose of the code \$\endgroup\$ – janos Mar 23 '17 at 17:38
1
\$\begingroup\$

For your current code:

  • Don't name variables float and number; name then for what they represent, not their datatype.
  • Don't use camelCase for methods; Ruby convention is to use snake_case

As for its functionality: There's a much easier way to get hours and minutes from a total number of minutes:

hours, minutes = total.divmod(60)

#divmod is basically a convenient shorthand for "quotient and remainder"; it's the same as this:

hours = (total / 60).floor
minutes = total % 60

just in one method.

Now, #divmod returns an array with 2 numbers in it, and in the code above I simply assign those two numbers to hours and minutes respectively.

If you then use #divmod again on those two but with 10 as the argument, you can get the "ones" and the "tens", the same way you just got "minutes" and "hours" by using 60 as your argument.

So say the total number of minutes is 119, i.e. 1h 59m, and you want to split both the hours and minutes, you can do this:

total
  .divmod(60)                    #=> [1, 59]
  .flat_map { |n| n.divmod(10) } #=> [0, 1, 5, 9]

Note however, that if you've got more than 99 hours, you could run into problems, as you'll only get the ones and tens; not the hundreds. E.g. 6000 minutes is 100 hours, but the above would return [10, 0, 0, 0] (sum 10), rather than all the individual digits, which would be [1, 0, 0, 0, 0] (sum 1).

This might be correct for the challenge still, since the readout is hh:mm, and thus doesn't say anything one way or the other about how 3-or-more hour-digits should be handled. The "digital sum" might be 10 or 1 - or zero if anything above 4 digits is just ignored. Or it could be d:hh:mm, and it'd be 4 days + 4 hours + 0 minutes = 8... there are just a lot of unknowns.

But if we assume that the input will stick to 2-digit hours, the code above will work without any ambiguity - all we need is the sum. For that you can use #reduce (aka #inject):

def late_ride(minutes)
  minutes
    .divmod(60)
    .flat_map { |n| n.divmod(10) }
    .reduce(:+)
end

and bingo.

But if the idea instead is to add up all the digits individually and the hours can have 3 or more digits, this, as mentioned, won't be quite right.

You can use #divmod repeatedly or recursively to get the digits of an arbitrary number of hours, but for this little challenge, I'll just suggest something I usually don't suggest: Convert the digital readout to a string, and get the digits that way. I usually try to avoid this, since it seems inelegant to treat a numeric value as a string only to get another numeric value. Just hey, it works, so:

total
  .divmod(60)  #=> [1, 59]
  .join        #=> "159"
  .chars       #=> ["1", "5", "9"]

From here, you can use #reduce again to get the sum (just call to_i to get actual numbers from the strings). And then this is your method:

def late_ride(minutes)
  minutes
    .divmod(60)
    .join
    .chars
    .reduce(0) { |sum, n| sum + n.to_i }
end

or, written another way:

def late_ride(minutes)
  minutes
    .divmod(60)
    .join
    .chars
    .map(&:to_i)
    .reduce(:+)
end
\$\endgroup\$
  • \$\begingroup\$ Thanks! This exactly the sort of response I was hoping to get. I knew there must be a much simpler way to do what I was doing. I did know about the convention to use snake_case, that was just a hold over from the Codefights problem(cut&paste). I knew a few of the methods, but I'd never seen .divmod before. The more I code the more I realize it's like Ninjutsu, knowing a method is like knowing how to punch or kick, it's putting combinations together that makes you a ninja. Thanks for the detail and helpful info. \$\endgroup\$ – Lenocam Mar 23 '17 at 17:45
  • \$\begingroup\$ @ReedMacConnell Well, while ninjas are cool, I'd prefer to think of it as vocabulary and expression since code is language, not a deadly fight :) I also updated my answer with a little more discussion and a simpler method that might work too, if some assumptions hold \$\endgroup\$ – Flambino Mar 24 '17 at 8:41

Not the answer you're looking for? Browse other questions tagged or ask your own question.