patternjavascriptMinor
"Stack Exchange page visits" alike implementation
Viewed 0 times
stackexchangealikepagevisitsimplementation
Problem
The objective is to increment the page visits by one. The user should not be able to do this by refreshing. The number of visits can be incremented only if the user's last visit was later than 10 minutes or none at all. This should work regardless of multiple users behind the same IP or router.
Does this need any improving?
Stack Exchange page views works like this.
Does this need any improving?
Stack Exchange page views works like this.
//Is data storage available?
if(typeof(Storage)!=="undefined")
{
var tenMinutes = 3600 * 10; //10 minutes
var now = new Date();
var visits = JSON.parse(localStorage.getItem('visits')); //Get visits array from local storage
var currentVisit = {id:$scope.ad.id, datetime:now};
var duplicate = false;
//There is no older visits, user is new here
if(!visits){
visits = [currentVisit];
addViewForID(currentVisit.id);
}
else{
//Check for duplicate visit
angular.forEach(visits, function(visit){
if (visit.id == currentVisit.id) {
duplicate = true;
var visitTime = new Date(visit.datetime);
var deltaTime = Math.abs(now.getTime() - visitTime.getTime());
var deltaDays = Math.ceil(deltaTime / (1000 * 3600 * 24));
if (deltaDays > 1 || (deltaDays == 1 && deltaTime > tenMinutes))
visit.datetime = now;
}
})
//First time visiting this page
if (!duplicate){
visits.push(currentVisit);
addViewForID(currentVisit.id);
}
}
//Save visits array in local storage
localStorage.setItem('visits', JSON.stringify(visits));
}
function addViewForID(id){
FData.incrementVisitsForID(id);
}Solution
My 2 cents:
-
I would not use an array of objects to represent the pages, I would use an object where each
-
Any enterprising individual is able to artificially inflate the counter, either by clearing the
-
Are you sure that 10 minutes = 3600 * 10, it seems wrong to me
I would counter propose something like this:
-
I would not use an array of objects to represent the pages, I would use an object where each
$scope.ad.id is a property and where the value is the last time visited. This would take out your loop and dupe management-
Any enterprising individual is able to artificially inflate the counter, either by clearing the
localStorage or by calling FData.incrementVisitsForID(id); in a loop from the console.-
Are you sure that 10 minutes = 3600 * 10, it seems wrong to me
I would counter propose something like this:
//Is data storage available?
if(typeof(Storage)!=="undefined")
{
var tenMinutes = 1000 * 60 * 10; //10 minutes
var now = (new Date()).getTime();
//Get visits array from local storage or if that fails, an empty object
var visits = JSON.parse(localStorage.getItem('visits')) || {};
//Get time of last visit, or 0
var lastVisit = visits[ $scope.ad.id ] || 0;
//Does it count?
if( now - lastVisit > tenMinutes)
{
FData.incrementVisitsForID( $scope.ad.id );
}
//Update visits for the next time
visits[ $scope.ad.id ] = now;
//Save visits array in local storage
localStorage.setItem('visits', JSON.stringify(visits));
}Code Snippets
//Is data storage available?
if(typeof(Storage)!=="undefined")
{
var tenMinutes = 1000 * 60 * 10; //10 minutes
var now = (new Date()).getTime();
//Get visits array from local storage or if that fails, an empty object
var visits = JSON.parse(localStorage.getItem('visits')) || {};
//Get time of last visit, or 0
var lastVisit = visits[ $scope.ad.id ] || 0;
//Does it count?
if( now - lastVisit > tenMinutes)
{
FData.incrementVisitsForID( $scope.ad.id );
}
//Update visits for the next time
visits[ $scope.ad.id ] = now;
//Save visits array in local storage
localStorage.setItem('visits', JSON.stringify(visits));
}Context
StackExchange Code Review Q#31560, answer score: 2
Revisions (0)
No revisions yet.