views:

38

answers:

2

Can anyone find any bugs or performance improvements here?

<a href='#' id='from_arrow_down' onclick="timeOffset(-1,'from_time');">Click me</a>

function timeOffset(offset,time_id)
{

  // javascipt utility that increments/decrements to the next 15 minute interval.
  // When user pushes a button, program will get time from input box, 
  // increment (offset=1) or decrement (offset=-1) to next (or previous)
  // quarter hour and out value back to input box.

  // This is used on handheld devices where keyboard is very small and input is     
  // difficult and time consuming.

  var pass_time = document.getElementById(time_id).value;
  var tempDate;

  // break in to hours and minutes 
  var HH = pass_time.substr(0,2);
  var MM = pass_time.substr(2,4);
  // dummy date
  try{
  tempDate = new Date("2000", "01", "01", HH, MM, "00", "0000");
  }
  catch(err)
  {
   alert("invalid time (HHMM)");
   return;
  }

  // dummy minutes
  var minutes = 999;

  // iterate until we have reached an inter
  while (minutes != 0 && minutes != 15 &&  minutes != 30 && minutes != 45){
   tempDate.setMinutes( tempDate.getMinutes() + offset );
   minutes = tempDate.getMinutes();   
   document.getElementById(time_id).value = cleanUp(tempDate.getHours()) + "" + cleanUp(tempDate.getMinutes());
  }

}

function cleanUp(d)
{
  if (d < 10){
   d = "0" + d;
  }
  return d;
}
+1  A: 

The while loop is very inneficient for this. I'd use setTimeout instead - call it every second if you need or every X increment you need.

Ryan Ternier
or setInterval (better yet, set this to a variable, so you can update the interval length easily)
Detect
I can only agree: it's very inefficient, and take 100% cpu and probably so the page is probably not updated.
some
The users liked the visual feedback when the button is pressed. It only runs when pressed, and then never more than 15 times. Is that the only "in so many ways"?
robert
Run 15 times? or run for 15 minutes? Unless I'm mistaken it looks like it's going to run for 15 minutes.
Ryan Ternier
Minutes is just a variable that contains a number between 0 and 59. I updated code with better explanation of what function is doing.
robert
Minutes is just a variable that contains a number between 0 and 59. The function runs in miliseconds. I updated the question with better explanation of what function is doing.
robert
A: 

Yeah, you can just calculate the distance to the next interval, then run the contents of the while loop on that value. You don't need to be constantly modifying the contents of the page with interim values for 'minutes'.

E.G., cast minutes to a float, divide by 15, use either the floor / ceil function for decrement / increment, then multiply by 15 again. That's just off the top of my head, it may not work for corner cases and such.

Sam Dufel