6
\$\begingroup\$

I am making an hours calculator app. It takes a start time, end time and time taken for lunch.

For the start and end time it takes a four-digit hh:mm time. For example: 10:20.

I have made a function which converts the time into a decimal, so 10:20 = 10.33. The function works, but I feel it looks a little heavy and wondered if anyone has any suggestions of how I could make it better...

const minuteConverter = time => {
    let h = Number(time.split(':')[0]);
    let m = Math.round((1 / 60 * (Number(time.split(':')[1])) + Number.EPSILON) * 100) / 100;
    let mConverted = Number(m.toString().split('.')[1])
    return Number(`${h}.${mConverted}`)
};
console.log(minuteConverter('10:20'))

The time must be output as a number to two decimal places. For example,

  • '10:20' >> 10.33
  • '9:45' >> 9.75
  • '15:33' >> 15.55
\$\endgroup\$
4
  • \$\begingroup\$ I believe you mean 10:20 in the last line, not 10.20? \$\endgroup\$
    – Tasos K.
    Sep 15, 2020 at 9:46
  • \$\begingroup\$ Apologies, that's correct! \$\endgroup\$ Sep 15, 2020 at 10:18
  • \$\begingroup\$ As I read your code, '10:75' >> 10.25, while all the answers will produce 11.25. You might want to think about which you want for such a denormalized time value. \$\endgroup\$
    – David G.
    Sep 16, 2020 at 1:07
  • 1
    \$\begingroup\$ are we just ignoring leap seconds like they arent a thing? \$\endgroup\$
    – Ewan
    Sep 16, 2020 at 15:11

4 Answers 4

15
\$\begingroup\$
  • Use const instead of let to declare variables if the value doesn't change.

  • You are executing time.split(':') twice.

  • A short method to convert a string to a number is the unary +.

  • JavaScript has the toFixed() method to format a number to a fixed number of digits:


function minuteConverter(time) {
   const [h, m] = time.split(':');
   const value = +h + +m / 60;
   return value.toFixed(2);
}
\$\endgroup\$
6
  • 2
    \$\begingroup\$ +h + +m / 60 - that second plus is redundant, because the divide operator converts strings too. \$\endgroup\$ Sep 16, 2020 at 5:38
  • \$\begingroup\$ @RoToRa Thankyou very much, that exactly what I'm talking about. I didn't know variables could be assigned like this either [h, m]. Big help, thankyou! \$\endgroup\$ Sep 16, 2020 at 8:08
  • 3
    \$\begingroup\$ I would probably convert in the first line with map, to make the intention more explicit, since all the + signs in the second row can be a little bit confusing: const [h,m] = time.split(':').map(s => +s) and then just const value = h + m/60 \$\endgroup\$
    – Falco
    Sep 16, 2020 at 9:11
  • \$\begingroup\$ Is there any reason you prefer +h++m over the arguably more telling Number(h)+Number(m) ? They seem to provide the exact same behaviour. But if someone reads your code and has to guess what you want to do is cast to a number then Number(h) seems to express that very clearly. \$\endgroup\$
    – Falco
    Sep 16, 2020 at 12:52
  • 1
    \$\begingroup\$ @Falco using the Number wrapper would likely be slower than using unary plus. For a trivial example like this it wouldn't make much difference but in a larger application where the function is called a large number of times then it could have considerable differences. I know jsPerf isn't currently working otherwise I would include a link to an example but This post explains more. \$\endgroup\$ Sep 16, 2020 at 15:27
6
\$\begingroup\$

Simpler technique

Per answers to this identical question from six years ago on stack overflow the formula for converting the minutes doesn’t need to be so complex. Many of the answers there use parseInt() to parse numbers from the strings but the unary plus operator + can be used instead for faster operation and simpler syntax (refer to answers to parseInt vs unary plus, when to use which? for more context).

The minutes can be divided by sixty and added to the number of hours to create a Number. The method Number.toFixed() can be used to limit floating point numbers to a specified number of digits.

ES-6 Variable declarations

Because ES6 keywords like let are used array destructuring can be used to assign the values for h and m in one expression which avoids extra calls to the split() method. This allows for const to be used, which is a good habit to help avoid accidental re-assignment.

Extra terminator after arrow function block

A multi-line arrow function expression does not require a semi-colon after the body but it doesn't hurt to have one at the end- especially if you are not familiar with the rules of Automatic semicolon insertion.

Simplified code

Here is one way it could be simplified.

const minuteConverter = time => {
    const [h, m] = time.split(':');
    return (+h + (+m/60)).toFixed(2);
};
['9:45', '10:03', '10:20', '15:33'].forEach(
    time => console.log(time, ' >> ', minuteConverter(time))
)

\$\endgroup\$
1
  • \$\begingroup\$ Much appreciated Sam, I did see that example from 6 years ago but I figured there had got to be some new method utilising ES6 features. Much appreciated, really concise. \$\endgroup\$ Sep 16, 2020 at 8:10
4
\$\begingroup\$

Better is relative! Lots of ways to write that code.

Though I personally prefer terse and precise code like other answers have highlighted, (here is my take on that):

const minuteConverter = time => [time.split(':')]
  .map(([hour, minute]) => +(+hour + +minute / 60).toFixed(2))[0]

The pluses could get a little hectic so here is a more descriptive/arithmetic dense version as well.

const minuteConverter = time => [time.split(':')]
  .map(([hh, mm]) => [+hh, mm / 60])
  .reduce((hundred, [hours, minutes]) => Math.round((hours + minutes) * hundred) / hundred, 100)
\$\endgroup\$
8
  • \$\begingroup\$ complex and confusing, without visible benefit for me. Wrapping an array in another array, just to use set-operators seems unnecessary, most of the other answers seem a lot clearer, use less code and are less complex. \$\endgroup\$
    – Falco
    Sep 16, 2020 at 9:33
  • \$\begingroup\$ Interesting take. Can you explain what is confusing you about the code? Is it a lack of familiarity with the concepts, the methodology? I assume you mean array methods when referring to set operators. That is not exactly the reason although doing so permits the style being used in the answer. I also am surprised that you feel there is more code (at least with the first take). Do you mean more characters, steps, lines? What do you mean when saying other answers have 'less code'? This would be helpful to understand on my part! Thanks for the feedback! \$\endgroup\$
    – santanaG
    Sep 16, 2020 at 12:02
  • \$\begingroup\$ Confusing is for me seeing an array operator, which is used for working on lists of elements being used on a single element. It would be like writing a for-loop with a single hardcoded iteration. And with "more code" I mean more elements (e.g. I count one variable, one operator one set of braces as one element each, irrespecitve of the length of the variable or keyword). \$\endgroup\$
    – Falco
    Sep 16, 2020 at 12:41
  • \$\begingroup\$ And the reduce version is just hard to digest - you assign 100 to the "sum" parameter of reduce and then use the variable name hundred, which seems to be done because there is no real use for reduce - you could just as well have used another map. For me it is a clear sign that map/reduce are not the right tools for operating on something which is not list-like (not an array/map/set/vector) \$\endgroup\$
    – Falco
    Sep 16, 2020 at 12:47
  • \$\begingroup\$ That is essentially making the assumption that the abstraction level that an array operator provides is solely used for arrays. Map is not exclusive in any way to lists but to functors, which JS arrays just happen to be. The reduce method is a very useful method that provides a map that can also return the contents of its array instead of the array with its contents. I could have easily mapped and used [0] to access but it would require I introduce one more 'magic' number to do so. Hence the choice. This was quick though so I guess I could have separated it into more steps for clarity. \$\endgroup\$
    – santanaG
    Sep 17, 2020 at 2:52
3
\$\begingroup\$

Use + instead of Number() on ES6.

Use const instead of let or var if the value for this variable not going to change.

Use a deconstructing assignment to create variables from the array let [h,m] = time.split(/[.:]/).

This will accept 10:30 or 10.30, also in case the time is .30 will add 0 for hours variable.

const timeStringToFloatMm = time => {
  let [h,m] = time.split(/[.:]/);
        h = h || 0;
  return (+h + +m / 60).toFixed(2);
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Is there any reason to use "+" instead of number? The stack-overflow question comparing various Methods of string to number conversion indicated Numer() and unary plus have the same behaviour and performance for all cases. And I feel especially with Number(h) + Number(m)/60 the intention becomes a lot clearer to the reader, whereas '+h++m' doesn't tell the story as clearly that the intention is in fact to case "h" and "m" to Numbers. \$\endgroup\$
    – Falco
    Sep 16, 2020 at 12:50
  • \$\begingroup\$ That is your choice, ES6 bring a new feature if you want use it. To get familiar/comfortable with any new syntax you have to use it many times. \$\endgroup\$
    – Moamen
    Sep 16, 2020 at 20:08

Your Answer

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

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