2
\$\begingroup\$

I have a variable containing a string in this format "Size-XS|Size-XL|Color-Red|Color-Green". When i process it the final result is "Variations: Size: XS, XL, Color: Green, Red. The code for this is like below.

$variations = explode("|", $search_db_row['variations']);
 foreach ($variations as $variations_arr1) {
  $variations_arr1_exp = explode("-", $variations_arr1);
  $variations_arr[$variations_arr1_exp[0]][] = $variations_arr1_exp[1];
 }
foreach ($variations_arr as $variations_arr2k => $variations_arr2v) {
 $variation.= $variations_arr2k.": ";
 foreach ($variations_arr2v as $variations_arr2_value) {
  $variation.= mb_strtolower($variations_arr2_value, "utf-8").", ";
 }
}

Is there a simpler way to achieve this? I think there are too much foreach's but I'm not sure how can I do it with less code.

\$\endgroup\$
3
\$\begingroup\$

This code is not very easy to follow.

(I am going to assume that the variables $search_db_row['variations'] and $variation where previously initialized)

Firstly, Your spacing is not very clear: This way:

$variations = explode("|", $search_db_row['variations']);

foreach ($variations as $variations_arr1) {
    $variations_arr1_exp = explode("-", $variations_arr1);
    $variations_arr[$variations_arr1_exp[0]][] = $variations_arr1_exp[1];
}

foreach ($variations_arr as $variations_arr2k => $variations_arr2v) {
   $variation.= $variations_arr2k.": ";
   foreach ($variations_arr2v as $variations_arr2_value) {
      $variation.= mb_strtolower($variations_arr2_value, "utf-8").", ";
   }
}

makes it easier to see what is connected to what.

Secondly, consider changing the variable names. Something like this:

$initial_variations = explode("|", $search_db_row['variations']);

foreach ($initial_variations as $initial_variations) {
    $each_variation = explode("-", $initial_variations);
    $variations_arr[$each_variation[0]][] = $each_variation[1];
}

foreach ($variations_arr as $variation_types => $variations_values) {
   $variation.= $variations_types.": ";
   foreach ($variations_values as $variations_value) {
      $variation.= mb_strtolower($variations_value, "utf-8").", ";
   }
}

Makes it easier to follow because the variable names themselves tell me what is happening.

As far as reducing the code/getting rid of the foreach's, you could try like this:

foreach ($initial_variations as $initial_variations) {
    $each_variation = explode("-", $initial_variations);
    if ($previous_type != $each_variation[0]){
        $variation .= $each_variation[0].": ";
    }
    $variation .= mb_strtolower($each_variation[1], "utf-8").", ";
    $previous_type = $each_variation[0];
}

two more points (homework):

  • you have a trailing comma at the end of each group (this prints Size: xs, xl, Color: red, green, instead of Size: xs, xl Color: red, green)

  • this could use a comment (or two)

\$\endgroup\$
  • \$\begingroup\$ First thank you for your answer. Interesting is that when the string is in this way "Size-XS|Color-Red|Size-XL|Color-Green" the result will be "Size: XS, Color: Red, Size: XL, Color: Green. This can be fixed by ordering the string the right way so no problem there. For the variable names yes, I have to start naming them more accordingly also the spacing so the code can be more readable. The coma at the end of each group separates them but I think I should remove it. Notes taken. \$\endgroup\$ – Luchezar Jul 28 '16 at 17:02

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.