patternjavascriptMinor
setTimeout and setInterval
Viewed 0 times
andsetintervalsettimeout
Problem
Problem statement
After 10 sec of execution of a program, start reading the property descriptors of each property of an object and continue reading all remaining properties each, every 3 seconds until done.
Solution
Can we improve the solution?
Are the naming conventions looking fine?
After 10 sec of execution of a program, start reading the property descriptors of each property of an object and continue reading all remaining properties each, every 3 seconds until done.
Solution
function Timer(obj, startTime, interval) {
var PDO ={
obj:{value: obj, enumerable: true, writable:false, configurable:false},
startTime: {value:startTime, enumerable:true, writable:false, configurable:false},
interval: {value:interval, enumerable:true, writable:false, configurable:false}
};
Object.defineProperties(this, PDO);
}
Timer.prototype.readProperties = function(){
var props = Object.getOwnPropertyNames(this.obj);
var self = this;
function readPDO(){
console.log(Object.getOwnPropertyDescriptor(self.obj, props[0]));
props.shift();
if (props.length === 0) {
clearInterval(self.clearID);
}
}
setTimeout(function(){
self.clearID = setInterval(function(){
readPDO();
},
self.interval
);
},
self.startTime
);
}
var t = new Timer({x:1, y:2}, 10000, 3000);
t.readProperties();Can we improve the solution?
Are the naming conventions looking fine?
Solution
Describe your problem better
While this may seem not be a thing to be pointing out in a code review it is important to describe your problem in a nice way.
I would describe it as follows:
I want to yield each property of an object every three seconds. However the first one should be delayed by 10 seconds.
Separation of concerns
You are mixing the time repeating problem with the property yielding.
You also don't allow a way to specify a different strategy for what to do whenever another property comes (in this case that would be the console.log).
EDIT
While my approach is sligthly better then yours there is still mixed code. Again
Solution: implement iterator pattern.
While this may seem not be a thing to be pointing out in a code review it is important to describe your problem in a nice way.
I would describe it as follows:
I want to yield each property of an object every three seconds. However the first one should be delayed by 10 seconds.
Separation of concerns
You are mixing the time repeating problem with the property yielding.
You also don't allow a way to specify a different strategy for what to do whenever another property comes (in this case that would be the console.log).
function setIntervalDelayed(delay, interval, cb){
setTimeout(function(){
if(cb()){
setIntervalDelayed(interval, interval, cb);
}
}, delay);
}
function yieldProperties(obj, delay, interval, cb){
var props = Object.getOwnPropertyNames(obj);
setIntervalDelayed(delay, interval, function(){
if(props.length == 0) return false;
cb(obj[props[0]]);
props.shift();
return props.length > 0;
})
}
yieldProperties({x:0, y:1, z:2}, 1000 * 10, 1000 * 3, console.log);EDIT
While my approach is sligthly better then yours there is still mixed code. Again
yieldProperties is doing more then it should, it still has that depedency of the delay and interval arguments, that don't really fit there.Solution: implement iterator pattern.
function setIntervalDelayed(delay, interval, cb){
setTimeout(function(){
if(cb()){
setIntervalDelayed(interval, interval, cb);
}
}, delay);
}
function iterate(arr, cb){
var i = 0;
return function(){
if(i >= arr.length) return false;
cb.apply(arr[i]);
++i;
return true;
};
}
function yieldProperties(obj, cb){
return iterate(Object.getOwnPropertyNames(obj), function(){
cb({
name: this.toString(),
value: obj[this]
});
});
}
setIntervalDelayed(10 * 1000, 3 * 1000, yieldProperties({x:0, y:1, z:2}, console.log.bind(console)))Code Snippets
function setIntervalDelayed(delay, interval, cb){
setTimeout(function(){
if(cb()){
setIntervalDelayed(interval, interval, cb);
}
}, delay);
}
function yieldProperties(obj, delay, interval, cb){
var props = Object.getOwnPropertyNames(obj);
setIntervalDelayed(delay, interval, function(){
if(props.length == 0) return false;
cb(obj[props[0]]);
props.shift();
return props.length > 0;
})
}
yieldProperties({x:0, y:1, z:2}, 1000 * 10, 1000 * 3, console.log);function setIntervalDelayed(delay, interval, cb){
setTimeout(function(){
if(cb()){
setIntervalDelayed(interval, interval, cb);
}
}, delay);
}
function iterate(arr, cb){
var i = 0;
return function(){
if(i >= arr.length) return false;
cb.apply(arr[i]);
++i;
return true;
};
}
function yieldProperties(obj, cb){
return iterate(Object.getOwnPropertyNames(obj), function(){
cb({
name: this.toString(),
value: obj[this]
});
});
}
setIntervalDelayed(10 * 1000, 3 * 1000, yieldProperties({x:0, y:1, z:2}, console.log.bind(console)))Context
StackExchange Code Review Q#116826, answer score: 3
Revisions (0)
No revisions yet.