Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

A helper function from this post on Stack Overflow this post on Stack Overflow could improve that:

A helper function from this post on Stack Overflow could improve that:

A helper function from this post on Stack Overflow could improve that:

added 1161 characters in body
Source Link
janos
  • 112.9k
  • 15
  • 154
  • 396

A helper function from this post on Stack Overflow could improve that:

function vsprintf_named($format, $args) {
 $names = preg_match_all('/%\((.*?)\)/', $format, $matches, PREG_SET_ORDER);
 $values = array();
 foreach($matches as $match) {
 $values[] = $args[$match[1]];
 }
 $format = preg_replace('/%\((.*?)\)/', '%', $format);
 return vsprintf($format, $values);
}

Like this:

$query = vsprintf_named(
 "SELECT u.*, COALESCE(c2.name,u.state) AS real_state, c.name AS real_country, bc.alpha3 AS country_iso, bs.code AS state_iso 
 FROM %(main).users AS u 
 INNER JOIN %(main).countries AS c ON u.country=c.id 
 LEFT JOIN %(main).countries AS c2 ON u.state=c2.id 
 LEFT JOIN %(blesta).countries AS bc ON c.country_iso_code=bc.alpha2 
 LEFT JOIN %(blesta).states AS bs ON bc.alpha2=bs.country_alpha2 AND COALESCE(c2.name,u.state)=bs.name 
 WHERE u.active=1 AND u.mail_activated=1 AND u.blesta_id=0",
 array('main' => MAIN_DB, 'blesta' => BLESTA_DB));

A helper function from this post on Stack Overflow could improve that:

function vsprintf_named($format, $args) {
 $names = preg_match_all('/%\((.*?)\)/', $format, $matches, PREG_SET_ORDER);
 $values = array();
 foreach($matches as $match) {
 $values[] = $args[$match[1]];
 }
 $format = preg_replace('/%\((.*?)\)/', '%', $format);
 return vsprintf($format, $values);
}

Like this:

$query = vsprintf_named(
 "SELECT u.*, COALESCE(c2.name,u.state) AS real_state, c.name AS real_country, bc.alpha3 AS country_iso, bs.code AS state_iso 
 FROM %(main).users AS u 
 INNER JOIN %(main).countries AS c ON u.country=c.id 
 LEFT JOIN %(main).countries AS c2 ON u.state=c2.id 
 LEFT JOIN %(blesta).countries AS bc ON c.country_iso_code=bc.alpha2 
 LEFT JOIN %(blesta).states AS bs ON bc.alpha2=bs.country_alpha2 AND COALESCE(c2.name,u.state)=bs.name 
 WHERE u.active=1 AND u.mail_activated=1 AND u.blesta_id=0",
 array('main' => MAIN_DB, 'blesta' => BLESTA_DB));
Source Link
janos
  • 112.9k
  • 15
  • 154
  • 396

The first way is less error-prone. When you write like this:

"FROM " . MAIN_DB . ".users AS u " . 
"INNER JOIN " . MAIN_DB . ".countries AS c ON u.country=c.id " .

... it's easy to "forget" to add a trailing space on the previous line, which is necessary to separate the clauses of the statement.

This might be slightly better, though it might look somewhat awkward:

" FROM " . MAIN_DB . ".users AS u " . 
" INNER JOIN " . MAIN_DB . ".countries AS c ON u.country=c.id " .

Here, it's easier to see the space before the expressions, as they are all aligned to the left, your eyes don't have to scan the right ends to see if you put a space there.

With your first version, this problem doesn't exist: the embedded newlines ensure that the clauses are separated, you cannot accidentally forget whitespace in between, it's naturally there. Another advantage of the first version is that it's easier to copy and paste to a query runner program.

Finally, when embedding variables in long strings, I find the interruptions caused by concatenation quite annoying, so I prefer to use formatting with a template, like this:

$query = sprintf(
 "SELECT u.*, COALESCE(c2.name,u.state) AS real_state, c.name AS real_country, bc.alpha3 AS country_iso, bs.code AS state_iso 
 FROM %s.users AS u 
 INNER JOIN %s.countries AS c ON u.country=c.id 
 LEFT JOIN %s.countries AS c2 ON u.state=c2.id 
 LEFT JOIN %s.countries AS bc ON c.country_iso_code=bc.alpha2 
 LEFT JOIN %s.states AS bs ON bc.alpha2=bs.country_alpha2 AND COALESCE(c2.name,u.state)=bs.name 
 WHERE u.active=1 AND u.mail_activated=1 AND u.blesta_id=0",
 MAIN_DB, MAIN_DB, MAIN_DB, BLESTA_DB, BLESTA_DB);

The drawback of this approach is that now the table names are not so easily visible in their contexts, which can invite mistakes in the argument list.

default

AltStyle によって変換されたページ (->オリジナル) /