# Updating a clock display with the time

Originally, I was going to post a question about how to condense four repetitive functions into a single function, but I wound up figuring it out myself before I could even post it.

However, the original idea that I had was to break down some of the repetitive sections into their own function and use those new sub-functions as part of the main one. I'm still not sure how I would have been able to make that work if I hadn't figured out the bigger picture.

function GetClock(){
var nday=d.getDay(),nmonth=d.getMonth(),ndate=d.getDate(),nyear=d.getYear();
if(nyear<1000) nyear+=1900;
var nhour=d.getHours(),nmin=d.getMinutes(),nsec=d.getSeconds(),ap;

if(nhour===0){ap=" ";nhour=12;}
else if(nhour<12){ap=" ";}
else if(nhour===12){ap=" •";}
else if(nhour>12){ap=" •";nhour-=12;}

if(nhour<=9) nhour="0"+nhour;
if(nmin<=9) nmin="0"+nmin;
if(nsec<=9) nsec="0"+nsec;

document.getElementById('clockbox').innerHTML=""+tday[nday]+", "+tmonth[nmonth]+" "+ndate+", "+nyear+" "+nhour+":"+nmin+":"+nsec+"<span class='ampm'>"+ap+"</span>";
}


This is one of the sections that I was originally thinking about splitting off into its own function:

if(nhour===0){ap=" ";nhour=12;}
else if(nhour<12){ap=" ";}
else if(nhour===12){ap=" •";}
else if(nhour>12){ap=" •";nhour-=12;}


These were my two feeble attempts to make it work:

function ampm() {
if(nhour===0){ap=" ";nhour=12;}
else if(nhour<12){ap=" ";}
else if(nhour===12){ap=" •";}
else if(nhour>12){ap=" •";nhour-=12;}
}


And then this:

function ampm(nhr,ampm) {
if(nhr===0){ampm=" ";nhr=12;}
else if(nhr<12){ampm=" ";}
else if(nhr===12){ampm=" •";}
else if(nhr>12){ampm=" •";nhr-=12;}
}


But they both failed due to (what I'm assuming are) issues with scope that I'm not sure how to solve.

Theres a lot of things here.

Disclaimer:

Using time stuff in JavaScript native is very hard and doesn't guarantee the cross browser. Also there are 300 of implementations of the same thing, plz don't make a new well. Please use a library like http://momentjs.com

First of all, is a little hard to read so, I started, formatting the code:

    function GetClock(){
var nday= d.getDay(),
nmonth= d.getMonth(),
ndate= d.getDate(),
nyear= d.getYear();

if(nyear<1000)
nyear += 1900;

var nhour= d.getHours(),
nmin= d.getMinutes(),
nsec= d.getSeconds(),
ampm;

if(nhour === 0) {
ampm = " ";
nhour = 12;
} else if (nhour<12) {
amPmSymbol =  " ";
} else if(nhour===12) {
ampm= " •";
} else if(nhour>12) {
ampm = " •";
nhour-= 12;
}

if(nhour<=9)
nhour = "0" + nhour;

if(nmin<=9)
nmin = "0" + nmin;

if(nsec<=9)
nsec = "0" + nsec;

document.getElementById('clockbox').innerHTML =
tday[nday] + ", " +
tmonth[nmonth] + " " +
ndate+", " +
nyear + " " +
nhour + ":" +
nmin + ":" +
nsec + "<span class='ampm'>" + ampm + "</span>";
}


So, i can see some issues and repeated code on this.

• This function doesn't return anything. So, why is called as Get?, I recommend use updateTime
• I can split date format, from hour/time format and am/pm symbol (I guess).
• also, I eliminate the unused/usefulness variables from the clock creation. Now all additional functions return strings, so, can be concatenate with the template.

function updateTime(){

if(nyear<1000)
nyear += 1900;

document.getElementById('clockbox').innerHTML =
ndate + ", " +
nyear + " " + getTimeAsFormatString(aDate);
}

if(hour === 0) {
hour = 12;
} else if(hour>12) {
hour-= 12;
}

hour = hour<= 9  ? "0" + hour : hour;
mins = mins<= 9  ? "0" + mins : mins;
secs = secs <= 9  ? "0" + secs : secs;

return hour + ":" + mins  + ":" + secs +  "<span class='ampm'>" + getAmPmSymbol(hour) + "</span>";

}

function getAmPmSymbol(hour) {
return (hour >= 12) ? " •" : " ";
}

• "Create"? What is it creating? The clockbox element already exists; the function is just updating its text content. – 200_success Nov 7 '15 at 7:21

I don't see a need for a new function here just some simplification, unless you plan on reusing these smaller blocks.

that said here is an example:

if(nhour===0){ap=" ";nhour=12;}
else if(nhour<12){ap=" ";}
else if(nhour===12){ap=" •";}
else if(nhour>12){ap=" •";nhour-=12;}


Can be replaced with the following:

ap = nhour >= 12 ? '  •' : '  ';
nhour = nhour%12 ? nhour%12 : 12;


Also you attempts at functionalizing lacked returns. This isn't necessarily wrong but lack of scope make it hard to know