Colour-changing clock

Question

I am fairly new to javascript and I would like to write my code on a more readable and efficient way. Any critique of what I've written would be greatly appreciated, especially the section of hexValues.first = changeHex... Also, should I have two files going for this code? And if so, how should I best split it up?

(function() {
 var newTime = function () {
 var clock = document.querySelector('.clock');
 var clockText = document.querySelector('.clock-text');
 var date = new Date();
 var hours = date.getHours();
 var minutes = date.getMinutes();
 var seconds = date.getSeconds();
 var timer = date.getTime();
 clockText.innerHTML = hours + " : " + minutes + " : " + seconds;
 var yesterday = new Date()
 yesterday.setMilliseconds(0);
 yesterday.setSeconds(0);
 yesterday.setMinutes(0);
 yesterday.setHours(0);
 var dateDifference = date - yesterday; 
 var changeHexValue = function (number) {
 if (number === 10) {
 number = 'A'
 } else if (number === 11) {
 number = 'B';
 } else if (number === 12) {
 number = 'C';
 } else if (number === 13) {
 number = 'D';
 } else if (number === 14) {
 number = 'E';
 } else if (number === 15) {
 number = 'F';
 }
 return number
 }
 var hexValueStart = Math.round(dateDifference / 5400000)
 var hexValues = {
 first : Math.round(hexValueStart * .16667),
 second : Math.round(hexValueStart * .33333),
 third : Math.round(hexValueStart * .5),
 fourth : Math.round(hexValueStart * .66667),
 fifth : Math.round(hexValueStart * .83333),
 sixth : hexValueStart
 } 
 hexValues.first = changeHexValue(hexValues.first).toString();
 hexValues.second = changeHexValue(hexValues.second).toString();
 hexValues.third = changeHexValue(hexValues.third).toString();
 hexValues.fourth = changeHexValue(hexValues.fourth).toString();
 hexValues.fifth = changeHexValue(hexValues.fifth).toString();
 hexValues.sixth = changeHexValue(hexValues.sixth).toString();
 // 86.4 comes from taking 86,400,000 and dividing by 999,999
 // with the former being the amount of milliseconds in a day
 // and 999,999 being the best max value for hex colors
 // var hexInput = Math.round(dateDifference / 86.4)
 // var rightDiv = document.querySelector('.right');
 var svg = document.querySelector('svg');
 var hexHolder = hexValues.first + hexValues.second + hexValues.third 
 + hexValues.fourth + hexValues.fifth + hexValues.sixth;
 svg.style.fill = "#" + hexHolder;
 // 240,000 comes from taking 86,400,000 and dividing by 360
 // with the former being the amount of milliseconds in a day
 // and 360 being the best max value on the color wheel
 var clockColor = Math.round(dateDifference/240000);
 clockText.style.color = 'hsl(' + clockColor + ',100%,50%)';
 // 11.13 comes from 256/23 with 256 being the max value 
 // of a rgb value and 23 for the max value of the variable
 // hours
 var rgbValueHours = Math.round(hours * 11.13)
 // 4.339 comes from 256/59 with 256 being the max value 
 // of a rgb value and 59 for the max value of the variable
 // minutes/seconds
 var rgbValueMinutes = Math.round(minutes * 4.339)
 var rgbValueSeconds = Math.round(seconds * 4.339)
 var backDiv = document.querySelector('.back');
 backDiv.style.background = 'rgb(' + rgbValueHours + ',' + rgbValueMinutes +
 ',' + rgbValueSeconds + ')'
 };
 newTime();
 window.setInterval(newTime, 1000);
})();

• rolfl: You have included no information on what your code is supposed to do... how can a review tell you if you are doing it right? Please include a description of what the problem is that your code solves, and make the title represent that task.
• 200_success: This is so close to being a runnable demo! Please include your HTML as a Code Snippet (Ctrl-M).

Answer 1

Here's some simplified code. I made the following changes:

1. Use yesterday.setHours(0, 0, 0, 0); to set hours, mins, secs,ms to 0
2. Calculate the A-F hex value rather than a bunch of else if statements
3. Returns a string from changeHexValue() since that is always what is needed
4. Put the multiplication multiples in an array that you can loop over rather than copying code
5. Calculate the final hexHolder string directly rather than using the intermediate object with first through sixth properties

Most of these changes come from one of these things to look for:

• Any time you see copied code, look to leverage one line of that code in a common function or in a loop
• Any time you are applying the same code with several different inputs, look to use a loop over a data structure
• Any time you have an algorithmic if/else structure, look to turn that into a calculation
• Use intermediate results only when it makes the code simpler to understand, otherwise just calculate the final result in one calculation rather than several
• If you see a common operation being applied to the return result of a function every time you call that function, then look to either just incorporate that operation into the function itself or create another function that calls the original and then applies your extra operation (again avoiding repeated code).

And, here's the code with those changes:

(function() {
 var newTime = function () {
 var clock = document.querySelector('.clock');
 var clockText = document.querySelector('.clock-text');
 var date = new Date();
 var hours = date.getHours();
 var minutes = date.getMinutes();
 var seconds = date.getSeconds();
 var timer = date.getTime();
 clockText.innerHTML = hours + " : " + minutes + " : " + seconds;
 // changed to pass four arguments to setHours
 var yesterday = new Date();
 yesterday.setHours(0, 0, 0, 0);
 var dateDifference = date - yesterday; 
 // calculate char code rather than many else if statements
 // always return a string
 var changeHexValue = function (number) {
 if (number >= 10 && number <= 15) {
 // 65 is the ascii code for 'A'
 number = String.fromCharCode(65 + number - 10);
 }
 return number + "";
 }
 // put multiplication factors in an array and loop over it
 // call changeHexValue right here rather than separately later
 // compute final hex string rather than use intermediate object
 var hexValueStart = Math.round(dateDifference / 5400000);
 var multiples = [.16667, .33333, .5, .66667, .83333, 1];
 var hexHolder = "";
 for (var i = 0; i < multiples.length; i++) {
 hexHolder += changeHexValue(Math.round(hexValueStart * multiples[i]));
 }
 // 86.4 comes from taking 86,400,000 and dividing by 999,999
 // with the former being the amount of milliseconds in a day
 // and 999,999 being the best max value for hex colors
 // var hexInput = Math.round(dateDifference / 86.4)
 // var rightDiv = document.querySelector('.right');
 var svg = document.querySelector('svg');
 svg.style.fill = "#" + hexHolder;
 // 240,000 comes from taking 86,400,000 and dividing by 360
 // with the former being the amount of milliseconds in a day
 // and 360 being the best max value on the color wheel
 var clockColor = Math.round(dateDifference/240000);
 clockText.style.color = 'hsl(' + clockColor + ',100%,50%)';
 // 11.13 comes from 256/23 with 256 being the max value 
 // of a rgb value and 23 for the max value of the variable
 // hours
 var rgbValueHours = Math.round(hours * 11.13)
 // 4.339 comes from 256/59 with 256 being the max value 
 // of a rgb value and 59 for the max value of the variable
 // minutes/seconds
 var rgbValueMinutes = Math.round(minutes * 4.339)
 var rgbValueSeconds = Math.round(seconds * 4.339)
 var backDiv = document.querySelector('.back');
 backDiv.style.background = 'rgb(' + rgbValueHours + ',' + rgbValueMinutes +
 ',' + rgbValueSeconds + ')'
 };
 newTime();
 window.setInterval(newTime, 1000);
})();

Some other thoughts:

1. Some of the things you're doing in here could easily be more general purpose functions on their own that would make them more available for reuse in this or other projects such as changeHexValue() and the calculation of hexHolder and the background color. You might consider make those utility functions outside of this scope that could be used elsewhere.

2. The variables clock, clockText and svg could be pulled out of newTime() and up into your IIFE (I'm assuming their values don't change) so they don't have to be refound every second when newTime() is called.