Lucy Posted March 27, 2015 Share Posted March 27, 2015 I've written code that displays the current date and time on my home page. After a while, though, the browser completely freezes. This probably has something to do with using up memory? Though I don't understand how. If anyone could look at my code, I'd be grateful. Thanks! "use strict";var dp = document.getElementById("homedate");var tp = document.getElementById("hometime");var days = ['Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday', 'Saturday', 'Sunday'];var months = ['January', 'February', 'March', 'April', 'June', 'July', 'August', 'September', 'October', 'November', 'December']var origD = new Date();var seconds = origD.getSeconds();var rem = (60 - seconds) * 1000;function convertDate() { var day = origD.getDay(); var month = origD.getMonth(); var year = origD.getFullYear(); var date = origD.getDate(); switch(date) { case 1, 21, 31: date = date + 'st'; break; case 2, 22: date = date + 'nd'; break; case 3, 23: date = date + 'rd'; break; default: date = date + 'th'; break; }; return days[day - 1] + ', ' + months[month] + ' ' + date + ' ' + year;}function convertTime() { var hours = origD.getHours(); var minutes = origD.getMinutes(); if (minutes < 10) { minutes = '0' + minutes; }; if (hours > 12) { hours = hours - 12; return hours + ' : ' + minutes + ' pm'; } else { return hours + ' : ' + minutes + ' am'; };}window.onload = function() { dp.textContent = convertDate(); tp.textContent = convertTime();};function refreshTime() { origD = new Date(); tp.textContent = convertTime(); setInterval(refreshTime, 60000);}setTimeout(refreshTime, rem); Link to comment Share on other sites More sharing options...
justsomeguy Posted March 27, 2015 Share Posted March 27, 2015 It's because you're using setInterval inside refreshTime. setTimeout schedules something to run once, setInterval schedules something to run until you stop it. So each time you call refreshTime you are creating another interval. That's going to increase exponentially. First you call refreshTime, and it creates 1 interval. Then the next time it gets called it creates another, so now you have 2 intervals. Each of those will create another, then you'll have 4, then 8, then 16, then 32, etc. You only need one. Either replace the call to setTimeout with setInterval and remove the call to setInterval inside refreshTime, or replace the call to setInterval with setTimeout. Link to comment Share on other sites More sharing options...
Lucy Posted March 27, 2015 Author Share Posted March 27, 2015 Okay, I see. Thing is, it's set up so on the page load, it takes the number of seconds until the next minute and waits until they have passed before changing the time, and thereafter changes it every 60 seconds - so the time is accurate. Hm... if I assigned the interval to a variable, do you think that would solve it? It would replace the interval each time rather than add one then, right? Though now I write it out it seems silly... :/ Link to comment Share on other sites More sharing options...
davej Posted March 27, 2015 Share Posted March 27, 2015 Fix version 1... function refreshTime(t) { origD = new Date(); tp.textContent = convertTime(); setTimeout(function(){refreshTime(60000)}, t);}refreshTime(rem); Fix version 2... function refreshTime() { origD = new Date(); tp.textContent = convertTime();}function startTimeDisplay(){ refreshTime(); setInterval(refreshTime, 60000);}refreshTime();setTimeout(startTimeDisplay, rem); Link to comment Share on other sites More sharing options...
justsomeguy Posted March 27, 2015 Share Posted March 27, 2015 You could assign the interval to a variable if you want to clear it every time, but if you're going to do that might as well just use setTimeout so you don't have to clear it. Assigning the interval to the same variable isn't going to stop the previous interval. Link to comment Share on other sites More sharing options...
Lucy Posted March 28, 2015 Author Share Posted March 28, 2015 Thanks guys, it's fixed now Link to comment Share on other sites More sharing options...
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now