1
\$\begingroup\$

I am new to javaScript, I just coded a method where I can convert a minute to a time in hh:mm AP/PM. I was then displaying the time in a table. I was wondering if there is a better approach to this? Because my code seems quite messy, with many if and else statement.

function convertMinsToHrsMin(minutes) {
                var str = '';
                var h = Math.floor(minutes / 60);
                var m = minutes - (h * 60);
                h = h < 10 ? '0' + h : h;
                m = m < 10 ? '0' + m : m;
                if (h >= 12) {
                    if (h == 12) {
                        str = "PM";
                    }
                    else if (h >= 22) {
                        h = (h - 12);
                        str = "PM";
                    }
                    else {
                        h = '0' + (h - 12);
                        str = "PM";
                    }
                }
                else {
                    if (h == 0) {
                        h = 12;
                        str = "AM";
                    }
                    else {
                        str = "AM";
                    }                     
                }                          
                return h + ":" + m + " " + str;
            }

            startTime = convertMinsToHrsMin(720);
            endTime = convertMinsToHrsMin(1080); 

            alert(startTime);
            alert(endTime);

Here is a jsFiddle to work on: https://jsfiddle.net/Issaki1/txns3mhc/

\$\endgroup\$
3
\$\begingroup\$

I was wondering if there is a better approach to this?

Yes. Use a library. Using what's already done is always better than doing it yourself. ;)

The next best thing you could do is convert minutes into milliseconds and instantiate a Date object from it. From there, JS does most of the heavy lifting for you. Here's an example (not extensively tested, but you get the idea):

const pad = n => `0${n}`.slice(-2)

const convertMinsToHrsMin = mins => {
  const date = new Date(mins * 60 * 1000)
  
  // Need to be UTC since timestamps are universal
  // but normal time methods operate based on local time.
  const hours = date.getUTCHours()
  const minutes = date.getUTCMinutes()
  
  // If 0, as in 00:00 or 12:00, default to 12.
  const hours12 = hours % 12 || 12
  const period = hours >= 12 ? 'PM' : 'AM'
  
  return `${pad(hours12)}:${pad(minutes)}${period}`
}

startTime = convertMinsToHrsMin(720);
endTime = convertMinsToHrsMin(1080);

alert(startTime);
alert(endTime);

As for your code:

  • Use console.log instead of alert. I know alert is a quick way to visualize values, but getting the habit of using console.log will do wonders for your skin.
  • Avoid very deep and branchy conditionals. if statements don't do anything on its own and has to be mutating some variable. That means you define variables up top. There is a chance that you might miss a branch and leave that variable empty or contain an unwanted value.
  • Use a ternary for short conditionals, like determining AM and PM. It's short, and gets you into not mutating variables.
  • Use template literals to construct strings. This way, you don't need to break out strings and you always get a string (not a NaN should your expression fail you due to coercion).
  • And lastly, get familiar with the native APIs. They do lots of heavy lifting for you so you don't have to write it yourself.
\$\endgroup\$
0
\$\begingroup\$

This is your code simplified. A few points:

  • I am very skeptical on whether should you use a library for a ten lines function. But you should first check what js native has to offer before go your own way. Check around Date object and see what you find.
  • Do not mess numbers with strings and in general, do not mix data types even if the language allows that. That is why I use h_literal to form the string

    function convertMinsToHrsMin (minutes) {
     var meridiem = 'PM', 
      h = Math.floor (minutes / 60),
      m = minutes - (h * 60),        
      m_literal = m < 10 ? '0' + m : m,
      h_literal 
     ;
    if (h == 0) {
      h = 12;
      meridiem = "AM";
    }
    else if (h > 12) {
      h -= 12;
    } 
    else if (h != 12) {
      meridiem = "AM";
    }  
    h_literal = h < 10 ? '0' + h : h;                            
      return h_literal + ":" + m_literal + " " + meridiem;
    }    
    startTime = convertMinsToHrsMin(720);
    endTime = convertMinsToHrsMin(1080); 
    console.log(startTime);
    console.log(endTime);
    
\$\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.