2
\$\begingroup\$

I am trying to learn functional programming in JavaScript, and saw a little task on Twitter that I wanted to have a go at:

var durations = ["12:38", "6:36", "9:03", "8:34", "5:02", "6:54", "13:22", "4:41", "8:36", "21:58", "3:06", "10:46", "10:13", "12:54",
"14:00", "11:03", "16:03", "10:52", "24:53", "10:03", "11:49",
"15:47", "3:19", "2:06", "5:47", "1:03", "5:29", "5:47", "26:39"];

// Above is a list of video lengths, how long are all the videos together?
// write a function that will take the above array of string durations and convert it hours/mins/seconds
// You can use any JS you want - loops/map/reduce/etc...

It would be great to have some feedback on how this could be refined further or improved.

var hours = 0, minutes = 0, seconds = 0;

function sumArrayIndex(array, i, separator) {
  return array
    .map(x => x.split(separator)
      .map(c => { return parseInt(c) })
    )
    .map(x => { return x[i]; })
    .reduce((x, y) => { return x += y }, 0);
}

function minToSec(m) {
  return m * 60
}

function tallyTime(s) {
  var hrs = 0;
  var min = Math.floor(s / 60);
  var sec = parseInt(s % 60, 16);

  if (min > 59) {
    hrs = Math.floor(min / 60);
    min = min % 60;
  }

  hours += hrs;
  minutes += min;
  seconds += sec;
}

function outputTime() {
  return hours + ':' + minutes + ':' + seconds;
}

var tally = tallyTime(minToSec(sumArrayIndex(durations, 0, ':')) + sumArrayIndex(durations, 1, ':'));

console.log(outputTime(tally));
\$\endgroup\$
1
\$\begingroup\$

The global hours, minutes, seconds are icky. Many of the functions you've defined access and change them, in essence causing side-effects.

Besides, this seems over-engineered. The sumArrayIndex function is a strange mix of specific and generic. It's specific because it expects an array of strings with a delimiter and stuff that can parsed to integers and summed. But it's generic in that it stops there. It's already built for this very specific task, but doesn't go all the way. Instead you have to call it twice (and it has to split everything twice, etc.).

You have 3 tasks here:

  1. convert each time code to the smallest common unit (i.e. seconds),
  2. calculate the total,
  3. format it as HH:MM:SS.

Your sumArrayIndex does some of the first two, rather than just one of them, or all of both.

Also having a function like minToSec is a little much. It's literally just a multiplication. In this context, I'd say that 60 doesn't even count as a magic number. So no reason to wrap it in a function; just multiply.

There also some bugginess: Should the final total be, say, 2:00:01, your code will output "2:0:1" because it doesn't pad the numbers.

And lastly, you're parsing seconds as a base16 number in tallyTime? I don't know why you're using parseInt on something that's already an int, and I certainly don't know why you're expecting it to be base16.

Anyway, here's my (very different) take:

function sumDurations(durations) {
  return durations.reduce((sum, string) => {
    var mins, secs;
    [mins, secs] = string.split(":").slice(-2).map(n => parseInt(n, 10));
    return sum + mins * 60 + secs;
  }, 0);
}

function formatDuration(duration) {
  function pad(number) {
    return `${number}`.slice(-2);
  }

  let hours = duration / 3600 | 0;
  let minutes = duration % 3600 / 60 | 0;
  let seconds = duration % 60;
  let minsSecs = `${pad(minutes)}:${pad(seconds)}`;

  return hours > 0 ? `${hours}:${minsSecs}` : minsSecs;
}

Some notes:

  • It's two functions for the sake of clarity, though they could be combined or be placed inside an outer function to hide that implementation detail (like pad is nested inside formatDuration).
  • I've combined steps 1 & 2 in sumDurations.
  • I'm using some array destructuring to pull out the minutes and seconds after splitting the time code string.
  • Like your code, you'll get wrong results if the time code contains hours as well. Mine ignores them, yours treats them as minutes - and the minutes as seconds. Fixing this is an exercise left to the reader.
  • I'm using the | 0 trick, which does the same as Math.floor (for positive numbers that fit in 32-bit integers anyway).
  • I'm using ES6 string templates/interpolation to build the final string.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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