patternphpModerate
Using goto to wait on the result of a database query to change
Viewed 0 times
resultthewaitquerydatabasegotousingchange
Problem
This is a small personal project where most non-sensitive data is distributed to some globally hosted CDN servers, which store the JSON. Now one problem we've seen during testing is a few deadlocks during spike loading of Ajax resources at the precise time that the cache is being distributed to the cache servers. So I came up with this locking scheme. Basically it will loop until the
Is this ok to put into production? The environment is a REST server written in Slim Framework. I'm not smart enough to know why
cache-lock value from one of the slave db servers responds with a value of 1, where 1 represents readable and 0 represents a locked state.Is this ok to put into production? The environment is a REST server written in Slim Framework. I'm not smart enough to know why
goto is considered evil so I would like a different opinion. I'm also curious if I should choose an alternative to sleep(1). public function wait_lock()
{
$steps = 0;
restart:
if(! $this->is_enabled('cache-lock') ){
if($steps > 4){
// the following method is a short cut for throwing an exception.
$this->e('cache error. wait a few moments and try again. if the problem persists, contact the administrator.');
}else{
sleep(1);
$steps++;
goto restart;
}
}
}Solution
Besides the unjustified use of
Counting loops are usually best written as
goto, I also object to the use of the number 4 for an operation that you try up to 6 times.Counting loops are usually best written as
for loops.public function wait_lock() {
for ($tries = 0; $tries is_enabled('cache-lock')); ++$tries) {
sleep(1);
}
if (!$lock) {
// the following method is a short cut for throwing an exception.
$this->e('cache error. wait a few moments and try again. if the problem persists, contact the administrator.');
}
}Code Snippets
public function wait_lock() {
for ($tries = 0; $tries < 6 && !($lock = $this->is_enabled('cache-lock')); ++$tries) {
sleep(1);
}
if (!$lock) {
// the following method is a short cut for throwing an exception.
$this->e('cache error. wait a few moments and try again. if the problem persists, contact the administrator.');
}
}Context
StackExchange Code Review Q#79426, answer score: 10
Revisions (0)
No revisions yet.