3
\$\begingroup\$

I have a function that takes a time of minutes and seconds, and converts it to a string in the following way: '0:30' would become '30 Seconds', '1:30' would become '1 minute 30 seconds', '2:01' would become '2 minutes 1 second', etc. My function works fine, but I'm wondering if there are areas for improvement.

function formatTime(formattedTime) {
 let minStr, secStr;
 let minNum = formattedTime.split(':')[0];
 let secNum = formattedTime.split(':')[1];
 if (minNum === '1') {
 minStr = 'minute';
 } else {
 minStr = 'minutes';
 }
 if (secNum === '01') {
 secStr = 'second';
 secNum = '1';
 } else {
 secStr = 'seconds';
 }
 if (minNum === '0') {
 return `${secNum} ${secStr}`;
 } else if (secNum === '00') {
 return `${minNum} ${minStr}`;
 } else {
 return `${minNum} ${minStr} ${secNum} ${secStr}`;
 }
}
asked Jan 5, 2018 at 19:59
\$\endgroup\$
3
  • 2
    \$\begingroup\$ minNum is easily confused with "minimum number", why not simply write minutes instead? \$\endgroup\$ Commented Jan 5, 2018 at 21:15
  • \$\begingroup\$ @le_m I felt it would be too repetitive to assign a variable to to it's own name as a string i.e. minutes = 'minutes' \$\endgroup\$ Commented Jan 6, 2018 at 20:21
  • 1
    \$\begingroup\$ @AaronGoldsmith "Too repetitive" is arguably a synonym of "easily understood", which is not a bad thing; code is not literary prose. \$\endgroup\$ Commented Jan 8, 2018 at 15:12

1 Answer 1

3
\$\begingroup\$

I made a few changes to the function that I think improve readability. By using parseInt() on the seconds, I eliminated the need to later set the secNum from 01 to 1. Since we no longer need to change that, the minStr and secStr can be set with a ternary operator. I also used destructuring to clean up the splitting of the time.

To make it less error prone, you'd also want to make sure to do some sanity checks on the input, as mentioned in the comments.

All told, it ended up looking like this:

// Tests
console.log(formatTime('1:00'))
console.log(formatTime('1:01'))
console.log(formatTime('1:30'))
console.log(formatTime('0:30'))
console.log(formatTime('bad data'))
function formatTime(formattedTime) {
 // Set up minutes and seconds
 const [match, minNum, secNum] = formattedTime.match(/(\d+):(\d+)/) || [];
 
 // Make sure we're getting the data in correct
 if(!match) return false;
 // Get the correct minutes and seconds strings
 let minStr = (minNum === '1') ? 'minute' : 'minutes'
 let secStr = (parseInt(secNum) === 1) ? 'second' : 'seconds'
 // Return the formatted time
 if (parseInt(minNum) === 0) {
 return `${secNum} ${secStr}`;
 } else if (parseInt(secNum) === 0) {
 return `${minNum} ${minStr}`;
 } else {
 return `${minNum} ${minStr} ${parseInt(secNum)} ${secStr}`;
 }
}

This is making use of some ES6 additions, so if you want good browser support you'll need to make sure to run this through Babel.

answered Jan 5, 2018 at 21:01
\$\endgroup\$
12
  • 3
    \$\begingroup\$ damn, i was gonna write an answer but i got caught up in work stuff.. the only thing I was gonna say that you didn't mention is to sanitize the input.. if(!formattedTime.match(/\d+:\d+/)) return false; \$\endgroup\$ Commented Jan 5, 2018 at 21:03
  • 2
    \$\begingroup\$ @le_m Good point on the extraneous declaration for minStr and secStr, updated! \$\endgroup\$ Commented Jan 5, 2018 at 21:15
  • 2
    \$\begingroup\$ beware of using parseInt() without passing the radix.... "If the input string begins with "0", radix is eight (octal) or 10 (decimal). Exactly which radix is chosen is implementation-dependent. ECMAScript 5 specifies that 10 (decimal) is used, but not all browsers support this yet." \$\endgroup\$ Commented Jan 5, 2018 at 22:29
  • 2
    \$\begingroup\$ Looks good. Just a couple of questions: why use let instead of const? why parseInt the seconds but not the minutes? \$\endgroup\$ Commented Jan 6, 2018 at 0:31
  • 1
    \$\begingroup\$ @le_m I thought I removed that redundancy, but apparently not. Stripped that out. \$\endgroup\$ Commented Jan 8, 2018 at 15:06

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.