3
\$\begingroup\$

I use 'GROUP_CONCAT' to get comma separated lists of a person's educational background. Included in those lists are the type of degree, major, issuing establishment, and year received. degreeType is a required field and cannot empty. Prior to looping, I split the lists into arrays, like so:

$degreeType = explode(",", $degreeType);
$major = explode(",", $major);
$issuingEstablishment = explode(",", $issuingEstablishment);
$yearReceived = explode(",", $yearReceived);

I can then do a foreach loop and echo out the information returned. However, since explode() returns an array, even if it's empty, I have to check it first before starting the foreach. As shown below:

$counter = 0;
//echo out education
if($degreeType[0]!=""){
  foreach($degreeType as $dType) {
    if($counter == 0) $info .= "<h2>Educational Background</h2><ul>";
    $info .= "<li>$dType - ";
    $info .= ($major[$counter]!="") ? "$major[$counter], " : "";
    $info .= $issuingEstablishment[$counter];
    $info .= ($yearReceived[$counter]!="") ? ", $yearReceived[$counter]" : "";
    $info .= "</li>";
    $counter++;
  }
  $info .= "</ul>";
}

Alternatively, I could run a check prior to separating the lists. Either solution seems inelegant. Is there a cleaner/faster/better way to split the lists and loop through them, but only if they actually contain information? Initially I was going to dabble with using an object, but mysqli doesn't support that with prepared statements. Any advice is much appreciated!

\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

Your script is alright, there are just some minor issues:

1.) If string is enclosed inside "" then PHP interpreter checks whether there are some variables or not, if string is enclosed inside '' PHP interpreter does not check it.

2.) You forgot to declare $info variable, it is always good to declare your variables before doing some operations on them like .=.

3.) I don't like using this shorter syntax, because it can be source of bugs which are hard to find, in my opinion it is usually better to use {}:

if($counter == 0) $info .= "<h2>Educational Background</h2><ul>";

4.) You don't need to repeat $info .= on every line, it can be replaced by ..

I rewrote the script in respect to previously mentioned points:

$degreeType = explode(',', $degreeType);
$major = explode(',', $major);
$issuingEstablishment = explode(',', $issuingEstablishment);
$yearReceived = explode(',', $yearReceived);

$counter = 0;
$info = '';
if($degreeType[0] != ''){
    foreach($degreeType as $dType) {
        if(!$counter) {
            $info .= '<h2>Educational Background</h2><ul>';
        }

        $info .= "<li>$dType - "
            . (($major[$counter] != '') ? "$major[$counter], " : '')
            . $issuingEstablishment[$counter]
            . (($yearReceived[$counter] != '') ? ", $yearReceived[$counter]" : '')
            . '</li>'
        ;

        $counter++;
    }
    $info .= "</ul>";
}
\$\endgroup\$
1
  • \$\begingroup\$ Oops! I forgot to copy-n-paste the info initialization, but it was in the original. Thanks for the tips, incorporating them now! \$\endgroup\$ Apr 23, 2014 at 20:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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