JS Clock widget

I want to get some best practices and code corrections.

JS

function clock(){
  var currentTime = new Date()
  var hours = currentTime.getHours()
  var minutes = currentTime.getMinutes()
  var seconds = currentTime.getSeconds()
  var timeOfDay = ( hours  12 ) ? hours - 12 : hours;
  hours = ( hours === 0 ) ? 12 : hours;
  minutes = ( minutes < 10 ? "0" : "" ) + minutes;
  seconds = ( seconds < 10 ? "0" : "" ) + seconds;
  var place = (hours + ":" + minutes + ":" + seconds + timeOfDay);
  document.getElementById('clock').innerHTML = place;
  setTimeout(clock,1000);
};
clock();

CSS

#clock {
  font-family:arial,helvetica;
  background:#FFF;
  box-shadow:3px 3px 5px 6px #ccc;
  width:86px;
  -webkit-box-shadow: 0 10px 6px -6px #777;
  -moz-box-shadow: 0 10px 6px -6px #777;
  box-shadow: 0 10px 6px -6px #777;
}

HTML

Time

initial findings:

You aren't using the init function, so why keep it. Whatever code you don't need: get rid of it.

Your clock function calls itself sort-of recursively with a 1 second interval, due to your using the setTimeout function.
You could easily avoid calling setTimout every time by simply changing that to setInterval:

function clock()
{
    //code here
}
var clockInterval = setInterval(clock, 1000);

The result of this is that, through the return value of setInterval (which is the interval's ID) you can stop the code from running whenever you need to:

//some event handler, for example a pauseBtn.addEventListener('click'...)
clearInterval(clockInterval);

which makes gives you more flexibility, and will make things easier when you want to debug or indeed add functionality later on.

The same can be done with an timeout, of course:

var tId = setTimeout(function()
{
    console.log('~5 seconds have passed');
}, 5000);
clearTimeout(tId);//the callback won't ever be called, unless for some reason this statement takes more than 5 seconds to reach.

However, an interval's ID is the same throughout. If you decide to use setTimeout, you'll have to re-assign the return value of every setTimout call to the same variable, simply because each timeout can, and most likely will, have a new ID. For that reason alone I think setInterval is the better choice in your case.

in the function, you have this: document.getElementById('clock') DOM lookup that queries the DOM for the same element over and over again. The DOM API is what it is: clunky and slow, so wherever you can, saving on DOM queries is a good thing. The element in question is, as far as your function is concerned pretty much a constant, is it not?

Why not use a closure, so you can query the DOM once, and use the same reference over and over? Something like this:

var clock = (function(clockNode)
{//clockNode will remain in scope, and won't be GC'ed
    return function()
    {//the actual clock function
        //replace document.getElementById('clock') with
        clockNode.innerHTML = place;
    };
}(document.getElementById('clock')));//pass DOM reference here
var clockInterval = setInterval(clock, 1000);

However, it's worth noting that innerHTML is quite slow, looks dated and is non-standard. The "correct" way of doing things would be:

clockNode.replaceChild(document.createTextNode(place), clockNode.childNodes.item(0));

Which, admittedly looks a tad more verbose and over-complicates things, but just thought you'd like to know.

Speaking of over-complicating things, and as phyrfox rightfully pointed out, you're essentially building a locale time string from the Date instance. You are re-inventing the wheel, considering the Date object comes with a ready-made method for just such a thing: toLocaleTimeString. This dramatically simplifies our code:

clockNode.replaceChild(
    document.createTextNode((new Date()).toLocateTimeString()),
    clockNode.childNodes.item(0)
);

That's all you need!

here's a fiddle

Oh, and remember how I said that the interval ID returned by setInterval can be a useful thing to have?

Here's another fiddle that illustrates that point

The code you have can, therefore be re-written to be more standard-compliant and at the same time be made shorter. What you end up with is simply this:

var clock = (function (clockNode)
{
    return function()
    {
        clockNode.replaceChild(
            document.createTextNode(
                (new Date).toLocaleTimeString()
            ),
            clockNode.childNodes.item(0)
        );
    };
}(document.getElementById('clock')));
var clockInterval = setInterval(clock, 1000);

Conventions

Yes, semi-colons are optional in JS, but it is considered a good habit to write them nonetheless. That's an easy convention to follow, and it'll help you when you decide to learn, or have to switch to, other languages. Many of which see the semi-colon as being non optional.
It's a bit of a hang-up of mine, I admit, but try to adhere to the conventions as much as possible, like: avoiding too many var declarations in a block of code. You can easily compact them into a single statement, using a comma:

var currentTime = new Date(),
    hours = currentTime.getHours(),
    minutes = currentTime.getMinutes(),
    seconds = currentTime.getSeconds(),
    timeOfDay = ( hours < 12 ) ? "AM" : "PM";

You can do away with the temp var place all together, and can even move the var declarations to the IIFE which we've used to store the clockNode DOM reference in:

var clock = function(clockNode)
{
    var currentTime, hours, minutes, seconds, timeOfDay;
    return function(){};
}());

Pass your code through the rather pedantic JSLint tool never hurts.

It'll probably complain about your keenness on using the ternary operator, too, because, though I'm not particularly opposed to the odd ternary, you d

StackExchange Code Review Q#44955, answer score: 12