JavaScript: Optimize function to create a vertical timeline

I created a simple function to iterate through a for loop and display a vertical timeline. Can I get some opinions on what I've created so far?

Are there ways to improve/optimize what I've created so far?

Ideally, what I'm after are ways to minimize duplications, extraneous calls, etc.

Here's my working code: http://jsfiddle.net/bL25b/

function timeline( start, abbr, hours )
{
  var a = 0,
      start,
      abbr,
      hours,
      time = document.getElementById('timeline');

  hours = (!hours) ? 12 : hours;

  for(a = a + start; a <= hours + start; a++)
  {
    if(a > 12)
    {
      time.innerHTML += '<li>' + (a - 12) + ':00 ' +
                          ( abbr == 'AM' ? 'PM' : 'AM' ) +
                        '</li>';

      time.innerHTML += '<li>' + (a - 12) + ':30 ' +
                          ( abbr == 'PM' ? 'AM' : 'PM' ) +
                        '</li>';
    }
    else
    {
      time.innerHTML += '<li>' + a + ':00 ' +
                          ( abbr == 'AM' ? (a == 12) ? 'PM' : 'AM' : 'PM' ) +
                        '</li>';

      time.innerHTML += '<li>' + a + ':30 ' +
                          ( abbr == 'AM' ? (a == 12) ? 'PM' : 'AM' 开发者_StackOverflow: 'PM' ) +
                        '</li>';
    }
  }
}

timeline( 9, 'AM', 12 );

The function arguments that are accepted:

• start: start time (0-12)
• abbr: Am/PM abbreviation
• hours: number of hours to display

---

See the updated code:

function timeline( start, abbr, hours )
{
    var a = 0,
        abbr = abbr || 'AM',
        hours = hours || 12,
        time = document.getElementById('timeline'),
        timelineHTML = [];

    for (a = a + start; a <= hours + start; a++)
    {
        if (a % 12 === 0) {
            abbr = abbr === 'PM' ? 'AM' : 'PM';
        }

        timelineHTML.push('<li>' + (a % 12 === 0 ? 12 : a % 12) + ':00 ' + abbr + '</li>');
        timelineHTML.push('<li>' + (a % 12 === 0 ? 12 : a % 12) + ':30 ' + abbr + '</li>');
    }

    time.innerHTML = timelineHTML.join('');
}

timeline( 9, 'AM', 24 );

The most significant change is minifying the DOM operations — we store our future elements in an array and append them at once.

Secondly, I removed unnecessary and confusing if..else block. The change between 'AM' and 'PM' occures every 12 hours, so a simple modulus operatior will do.

The part where (a % 12 === 0 ? 12 : a % 12) might be still confusing, but it is kept to display 12:30 AM instead of 0:30 AM. You can change it to short (a % 12) if you want.

Finally, you used hours = (!hours) ? 12 : hours, while simple hours = hours || 12 is more readable.

---

Don't use innerHTML += every time, use an array to store the html partials, then use join method to join them to a string and assign the string to innerHTML

---

Instead of innerHTML , use:

1. createElement - to create new LI element

2. insertAfter - to insert it in the end

To add new node to existing one. That would be faster than adding innerHTML to existing innerHTML.

---

If you're going to touch an element's innerHTML, it's usually considered optimal to do so all at once. In addition, your existing for loop has to process the addition every time, and it's best to define the finish point of the loop beforehand.

var newHTML = '';
var finish = hours+start;

for(a=a+start; a <= finish; a++)
{
    if(a > 12)
    {
        newHTML += '<li>' + (a-12) + ':00 ' + ( abbr == 'AM' ? 'PM' : 'AM' ) + '</li>';
        newHTML += '<li>' + (a-12) + ':30 ' + ( abbr == 'PM' ? 'AM' : 'PM' ) + '</li>';
    }
    else
    {
       newHTML += '<li>' + a + ':00 ' + ( abbr == 'AM' ? (a==12) ? 'PM' : 'AM' : 'PM' ) + '</li>';
       newHTML += '<li>' + a + ':30 ' + ( abbr == 'AM' ? (a==12) ? 'PM' : 'AM' : 'PM' ) + '</li>';
   }
}

time.innerHTML += newHTML;

---

Everyone else covered the broad strokes (@Lapple especially), so here's a couple very minor low-hanging items:

hours = (!hours) ? 12 : hours;

Can be shortened to:

hours = hours || 12;

...and is a more readable idiom (I think) to most JavaScript programmers.

Edit re: your comment:

I don't know of a name for this particular idiom. Like most programming languages JavaScript uses short-circuit evaluation on boolean expressions, i.e. given X || Y if the interpreter can already tell the value of expression will be from X (because X is "truthy"), it never bothers to evaluate Y. So you could do something like true || delete_everything() confident that delete_everything would never be called. Likewise in X && Y if X is "falsy" then Y will never be evaluated.

This might all be old hat to you. What JavaScript does that's less common, though, is what you could call "last value" evaluation (this is the term the Wikipedia page uses but I'm not sure if it's a common term at all). Instead of returning true or false from a boolean expression as in Java, or 1 or 0 as in C, in JavaScript it just returns the last value evaluated.

Take the expression hours || 12. If hours is undefined (a falsy value in JavaScript), then the interpreter will just return the second operand (since in an OR the "truthiness" of the expression is always equal to the truthiness of the second operand when the first operand is falsy). However, if hours is truthy--say, 9 or "banana", then the expression will short-circuit after evaluating it and return that value. To repeat myself, but in code:

var hours; // declared but has the value undefined

console.log( hours || 12 ); // `hours` (undefined) is falsy
// => 12                    // so return the second operand

hours = 9;
console.log( hours || 12 ); // `hours` is truthy--short-circuit & return `hours`
// => 9

hours = "banana";
console.log( hours || 12 ); // `"banana"` is also truthy
// => "banana";

console.log( 12 || hours ); // `12` is truth--short-circuit & return `12`
// => 12

Incidentally, in languages like Ruby that have the ||= operator, there's an even shorter form of this idiom that's fairly common:

hours = nil          # `nil` is like `null`

hours = hours || 12  # just like in JavaScript
# => 12

# but the below is equivalent:

hours = nil

hours ||= 12
# => 12

So in Ruby it's not uncommon to see methods like this:

def foo(bar, baz = nil) # `baz` defaults to `nil` when no 2nd argument is given
  baz ||= get_default_baz()

  puts bar + baz        # `puts` is like `print` or `echo` in other languages
end

def get_default_baz     # a trivial example
  "WXYZ"
end

foo('ABC', 'DEF')
# => ABCDEF

foo('ABC')
# => ABCWXYZ

(End edit)

And here:

var a = 0,
    start,
    abbr,
    hours,
    time = document.getElementById('timeline');

hours = (!hours) ? 12 : hours;

...you're declaring hours on line 4 and then assigning it on line 7, when you could do both at the same time:

var a = 0,
    // ...
    hours = hours || 12
    // ...
;