patternphpMinor
Is there a more elegant solution than this spaghetti code without completely revamping the whole page?
Viewed 0 times
thisspaghettiwithoutsolutionthecompletelywholethanmorerevamping
Problem
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.
I can then do a
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
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 .= "Educational Background";
$info .= "$dType - ";
$info .= ($major[$counter]!="") ? "$major[$counter], " : "";
$info .= $issuingEstablishment[$counter];
$info .= ($yearReceived[$counter]!="") ? ", $yearReceived[$counter]" : "";
$info .= "";
$counter++;
}
$info .= "";
}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!Solution
Your script is alright, there are just some minor issues:
1.) If string is enclosed inside
2.) You forgot to declare
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
4.) You don't need to repeat
I rewrote the script in respect to previously mentioned points:
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 .= "Educational Background";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 .= 'Educational Background';
}
$info .= "$dType - "
. (($major[$counter] != '') ? "$major[$counter], " : '')
. $issuingEstablishment[$counter]
. (($yearReceived[$counter] != '') ? ", $yearReceived[$counter]" : '')
. ''
;
$counter++;
}
$info .= "";
}Code Snippets
$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>";
}Context
StackExchange Code Review Q#48003, answer score: 4
Revisions (0)
No revisions yet.