2
\$\begingroup\$

The below code is working correctly, however, I am not sure if the combination of while, foreach and implode is really necessary here or if the same can be achieved much simpler.

What I do here is fetching some data (text) from a db and the only thing I need is the results as a comma separated string.

Now even though this is working I was wondering if there is a direct way to get the results as a comma separated string since the arrays here are not needed for anything else.

$stmt = $conn->prepare("SELECT " . $languageFrm . " FROM TranslationsMain WHERE location LIKE ? ORDER BY sortOrder, " . $languageFrm);
$stmt->bind_param("s", $location);
$stmt->execute();
$result = $stmt->get_result();
$strTranslations = array();
while($arrTranslations = $result->fetch_assoc()){
    $translations[] = $arrTranslations;
}
foreach($translations as $translation){
    array_push($strTranslations, $translation[$languageFrm]);
}
echo implode(",", $strTranslations);
\$\endgroup\$
1
\$\begingroup\$

Instead of two loops + an array + implode, you could use a single loop, appending to a string the elements and a comma. For a non empty result, there will be a trailing unnecessary comma, so chop it off at the end.

Trying to move the formatting logic into the query is not a great idea for two reasons:

  • a database is not a formatting tool. Good separation of concerns suggests to avoid using the database for such purpose
  • a query with string concatenation will most probably sacrifice portability. Your current SQL is nicely portable: it will work fine in probably any rdbms. It would be good to keep it that way
\$\endgroup\$
  • \$\begingroup\$ I was just reading through this. So to make sure I get it right, within my while loop I could just say $translations .= $arrTranslations . "," with $translations now being a standard variable ? If so, how would I get rid of the trailing comma in this case ? Should I use $translations = substr($translations, 0, -1) for that ? \$\endgroup\$ – keewee279 Jun 27 '15 at 6:34
  • \$\begingroup\$ Thanks, so the substr function I added should work. How about what I suggested for the while loop ? Will that cover the rest ? \$\endgroup\$ – keewee279 Jun 27 '15 at 6:40
  • 1
    \$\begingroup\$ Yes, like that! \$\endgroup\$ – janos Jun 27 '15 at 6:42
  • 1
    \$\begingroup\$ If I'm reading your code right, you need $arrTranslations[$languageFrm] inside the loop. \$\endgroup\$ – janos Jun 27 '15 at 13:45
  • 1
    \$\begingroup\$ Never mind - I figured this out with what you suggested. Not sure what I had wrong there before, maybe it was a typo. Sorry for that and thanks again ! :) \$\endgroup\$ – keewee279 Jun 27 '15 at 14:23
2
\$\begingroup\$

Using one of the below queries removes the need to iterate over the result sets to construct the CSV strings. When a single result is expected, a ->fetch() instead of ->fetchAll() would do.

Conclusion: implode goes away, while and for loops go away the only iteration needed would be in case of multiple CSV result sets.

SELECT GROUP_CONCAT(columnName) FROM Table1; # for single column

SELECT CONCAT(columnA,",",columnB) from Table2; # for multi cols

You might have issues creating prepared statements for this; I don't know.

example for the first query:

<?php

$conn = new PDO("mysql:host=localhost;dbname=test", "root", "");

$stmt = $conn->query("SELECT GROUP_CONCAT(columnName) FROM Table1");

$result = $stmt->fetch(PDO::FETCH_ASSOC);

var_dump($result);
\$\endgroup\$
  • \$\begingroup\$ This looks very interesting, esp. the first option but how would I echo the result of this (as a comma separated string) if you say both the while and the for loop are not needed ? \$\endgroup\$ – keewee279 Jun 27 '15 at 13:35
  • \$\begingroup\$ edited the answer to answer your question \$\endgroup\$ – Alex Andrei Jun 29 '15 at 11:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.