convertToHHMM function

Question

Can someone help me make a more elegant solution to this function?

function convertToHHMM(info) {
var suffix='AM';
var hrs = parseInt(Number(info));  
var min = Math.round((Number(info)-hrs) * 60);
if (hrs >12) { hrs = hrs - 12; suffix='PM'; }
if (min < 10) {min='0'+min;}
return hrs + ':' + min + ' ' + suffix;
}

Answer 1

It's kinda hard to make it "more elegant", but it can certainly be made more correct.

Since you use Math.round, you can get a time like "3:60 PM" if you pass it 15.999. And I'm preeeetty sure there's no such thing.

There's also another bug. If you pass a value like 0.5, you should get "12:30 AM". But you don't. You get "0:30 AM".

And some nitpicking: First off, info is not a very descriptive name. It looks like you're trying to format a number of hours, with minutes as the decimals, so hours would be a better name. (Edit: or better yet fractionalHours as rolfl suggests, and which I forgot to credit.)

Also, instead of using parseInt (which is meant to turn a string into an integer), use Math.floor, which is what you actually mean: The number without any decimals. For the minutes, you could use the modulo operator (%) to get the remainder of dividing by 1 (i.e. the decimals) instead of subtracting. But either one works.

By the way, even if your input is a string like "12.3231" JavaScript's type coercion will treat it as a number, so you don't really need Number() anywhere, but it's fine to add it.

You can also do something slightly tricky by using modulo 12 on the hours, and default to 12 if the result it zero. That fixes the 0/12 bug mentioned above.

Also, fewer lines do not make things better/faster/more elegant. In this case, it just hurts readability. So use some linebreaks instead of having a line like if (hrs >12) { hrs = hrs - 12; suffix='PM'; }.

Lastly, use some indentation. It may just be a copy/paste thing when you created the question, but still.

I might do something like this, first converting everything to minutes and rounding that, then breaking it into hours and minutes afterward:

function convertToHHMM(fractionalHours) {
  var totalMinutes = Math.round(fractionalHours * 60) % (24 * 60),
      hours = Math.floor(totalMinutes / 60) % 12 || 12,
      minutes = totalMinutes % 60,
      suffix = totalMinutes >= 12 * 60 ? 'PM' : 'AM';

  if(minutes < 10) minutes = '0' + minutes;

  return hours + ':' + minutes + ' ' + suffix;
}

• totalMinutes rolls over after rounding, so passing in 23.999 is equivalent to passing in 0, and 27 becomes 3, etc.
• hours also roll over but start at 12 instead of zero, then 1, 2, 3 and so on.
• minutes should be straight-forward, and
• suffix basically looks at whether the totalMinutes are greater than or equal to noon-in-minutes.

Snippet with some test cases below (format borrowed from rolfl).

function original(info) {
var suffix='AM';
var hrs = parseInt(Number(info));  
var min = Math.round((Number(info)-hrs) * 60);
if (hrs >12) { hrs = hrs - 12; suffix='PM'; }
if (min < 10) {min='0'+min;}
return hrs + ':' + min + ' ' + suffix;
}


function reviewed(fractionalHours) {
  var totalMinutes = Math.round(fractionalHours * 60) % (24 * 60),
      hours = Math.floor(totalMinutes / 60) % 12 || 12,
      minutes = totalMinutes % 60,
      suffix = totalMinutes >= 12 * 60 ? 'PM' : 'AM';

  if(minutes < 10) minutes = '0' + minutes;

  return hours + ':' + minutes + ' ' + suffix;
}

var testCases = [
  ["12:00 AM", 0],          // midnight
  ["12:30 AM", 0.5],
  ["1:00 AM",  0.99999999],
  ["11:30 AM", 11.5],
  ["12:00 PM", 11.9999999], // noon
  ["12:00 PM", 12],
  ["11:30 PM", 23.5],
  ["12:00 AM", 23.9999999],
  ["12:00 AM", 24],
  ["1:00 AM",  25]
];

var data = "<table>";
data += "<tr><th>Input</th><th>Expected</th><th>Review func</th><th>Original func</th></tr>";
for(var i = 0, l = testCases.length ; i < l ; i++) {
  data += "<tr>";
  data += "<td>" + testCases[i][1] + "</td>";
  data += "<td>" + testCases[i][0] + "</td>";
  data += "<td>" + reviewed(testCases[i][1]) + "</td>";
  data += "<td>" + original(testCases[i][1]) + "</td>";
  data += "</tr>";
}
data += "</table>";

document.body.innerHTML = data;

body { font-family: monospace }
table { border-collapse: collapse }
td, th {
  padding: 0.2em 0.5em;
  border: 1px solid #ccc
}

Answer 2

This question really sucks, and it's a good example of why Code Reviews are important, and there is a lot to learn in it. I was very close to closing the code as "Does not work".

3 hours ago you were asked to give some clarification and context. You did not. So, let me show you how a programmer should present their code for review, because in there is a lesson too. This is approximately what your question should have looked like:

My function takes a time and formats the time in Hours and Minutes with a AM/PM suffix. The input time is in the format of a floating-point number, so, for example, 14.5 is 14 and a half hours after midnight, or 2:30PM

function convertToHHMM(info) {
    var suffix='AM';
    var hrs = parseInt(Number(info));  
    var min = Math.round((Number(info)-hrs) * 60);
    if (hrs >12) { hrs = hrs - 12; suffix='PM'; }
    if (min < 10) {min='0'+min;}
    return hrs + ':' + min + ' ' + suffix;
}

Why is that text introduction necessary? Well, for a start, your code does not tell us what info is. It could be a time-since-epoch, it could be seconds-since-start-of-day, it could be anything.

I had to struggle through WTF moments to understand why parseInt(Number(info)) was necessary. How was I supposed to know that info was already a Number, and not a String, or some other object, like a Date. I had to understand that the ParseInt was important not because the input was a String, but because it was a floating-point number and you wanted just the integral part.

OK, so, having done the hard work of understanding how your function works, and what the inputs and outputs are, this is how I would write it....

... but first, the bugs in your code.

1. what if someone gives you the input 5.9999. Your code will output 5:60AM which makes no sense. The Math.round() of the minutes is a problem. You want to get Math.floor().
2. your handling around noon is very problematic. You use hrs > 12 to test for noon, but, that should be hrs >= 12. As it stands, your code says the input 12.50 is 12:30AM when it should be 00:30PM.

This is approximately how I would have written your function, given the above... I have compared it with yours using some input values to test with.

function convertToHHMMX(fractionalHours) {
  var fHours = Number(fractionalHours);
  var hours = Math.floor(fHours);
  var minutes = Math.floor((fHours - hours) * 60.0);
  var fill = minutes < 10 ? '0' : '';
  var suffix = "AM";
  if (hours >= 12) {
    suffix = "PM";
    hours -= 12;
  }
  if (hours == 0) {
    // make 00:30AM / PM become 12:30AM / PM
    hours += 12;
  }
  return hours + ':' + fill + minutes + suffix;
}

function convertToHHMM(info) {
var suffix='AM';
var hrs = parseInt(Number(info));  
var min = Math.round((Number(info)-hrs) * 60);
if (hrs >12) { hrs = hrs - 12; suffix='PM'; }
if (min < 10) {min='0'+min;}
return hrs + ':' + min + ' ' + suffix;
}

var data = "YourFn     Monkey\n";
for (var t = 0; t < 24; t += 0.49999) {
  data += convertToHHMM(t) + "     " + convertToHHMMX(t) + "\n";
}
document.getElementById("output").innerHTML = data;

<pre id="output">Output</pre>

Answer 3

It looks like your code is essentially converting military time in number format to 12-hour time.

Suggestions

• Instead of parseInt() you can use bitwise right-shifting to efficiently strip decimals off a number. The advantages are both efficiency and avoiding potential problems when the input has a preceding 0. (EDIT: Flambino pointed out in the comments that you can also use the bitwise OR operator x | 0; to accomplish the same thing!)

  • use regular ol' bitwise rightshift x >> 0 for potentially negative values for x
  • use unsigned bitwise rightshift x >>> 0 when x is guaranteed to be positive

• Use the modulus operator % to get the remainder when a number is divided by another number.

  • You can use this to convert the hours to the range of 12, no matter how high it goes: hrs %= 12; Note that this will convert 12 to 0, so you'd need to check for 0 and change it to 12.
  • You can use this to get the decimal remainder for calculating the minutes: var min = (+info % 1)*60 >>> 0;
  • And finally, you can use this to determine whether the converted hour will be in the morning or the afternoon: var suffix = hrs % 24 >= 12 ? "PM" : "AM";

Here's an example using those suggestions:

function convertToHHMM(info) {
    var hrs = (+(info) >>> 0);
    var min = (+(info) % 1) * 60 >>> 0;
    var suffix = hrs % 24 >= 12 ? "PM" : "AM";
    hrs %= 12;
    if (hrs === 0) {
        hrs = 12;
    }
    return hrs + ":" + (min < 10 ? "0" : "") + min + " " + suffix;
}

Side Notes

+info is functionally equivalent to Number(info) but takes up less space; that's a personal preference rather than a recommendation. If you prefer Number() then stick with it.

Similarly, I used the ternary operator (condition ? ifTrue : ifFalse) syntax a few times to keep the code concise, but you may prefer If/Else statements for readability.

Working Example

Here's a code snippet to see it in action:

document.getElementById("submit").onclick = function() {
  var input = document.getElementById("input").value;
  document.getElementById("output").innerHTML = convertToHHMM(+input);
};

function convertToHHMM(info) {
  var hrs = (+(info) >>> 0);
  var min = (+(info) % 1) * 60 >>> 0;
  var suffix = hrs % 24 >= 12 ? "PM" : "AM";
  hrs %= 12;
  if (hrs == 0) {
    hrs = 12;
  }
  return hrs + ":" + (min < 10 ? "0" : "") + min + " " + suffix;
}

<input type="text" id="input" value="1.5" />
<input id="submit" type="button" value="submit" />
<div id="output" />