5
\$\begingroup\$

This a very simple function to expand the sizes between the given values.

For instance, if the input variable is "S-XL" then the function returns "S, M, L, XL".

Basically I'm not happy with this function. I think it's too complex for its purpose.

function sizexpander($sizes) {
    $sizearray=Array("XS", "S", "M", "L", "XL", "XXL", "XXXL");
    $size1=array_search(strstrb($sizes, "-"), $sizearray);
    $size2=array_search(substr(strstr($sizes, "-"),1), $sizearray);
    while ($size1 <= $size2) {
        $prodsizes.= $sizearray[$size1].", ";
        $size1++;
    }
    return substr($prodsizes, 0, -2);
}

function strstrb($h,$n){
    return array_shift(explode($n,$h,2));
}
\$\endgroup\$
6
\$\begingroup\$

You can use the function array_slice to take the subarray which contains the sizes from $size1 to size2. And then use implode to turn this subarray into a comma-separated string. This will simplify your algorithm.

You can also just use explode instead of substr and strstr to split the range string into its parts.

In addition I would recommend changing the variable names of $size1 and $size2. To me the name $size1 suggests that the variable contains the actual size while in fact it contains its index. Something like index_of_size1 or index_from might be more appropriate.

If I were to write this function, it'd look something like this:

function sizexpander($size_range) {
    $sizearray=Array("XS", "S", "M", "L", "XL", "XXL", "XXXL");
    list($size_from, $size_to) = explode("-", $size_range);
    $index_from = array_search($size_from, $sizearray);
    $index_to = array_search($size_to, $sizearray);
    $subsizes = array_slice($sizearray, $index_from, $index_to-$index_from+1);
    return implode(", ", $subsizes);
}
\$\endgroup\$
2
\$\begingroup\$

Option A:

function sizexpander($sizes) {
    $sizeArray = array("XS", "S", "M", "L", "XL", "XXL", "XXXL");
    list($minSize, $maxSize) = split('-', $sizes);

    $minIndex = array_search($minSize, $sizeArray);
    $maxIndex = array_search($maxSize, $sizeArray);
    return join(', ', array_slice($sizeArray, $minIndex, ++$maxIndex - $minIndex));
}

You can do this with array_filter but it's not as efficient.

If you're uncomfortable with array operations or find the code unclear (i.e. not self documenting) you can do this with a simple enough loop.

function sizexpander2($sizes) {
    $sizeArray = array("XS", "S", "M", "L", "XL", "XXL", "XXXL");
    list($minSize, $maxSize) = split('-', $sizes);
    $tmp = array();

    foreach($sizeArray as $size) {
        if($size == $minSize) {
             $tmp[] = $size;
        } else if(count($tmp)) {
             $tmp[] = $size;
             if($size == $maxSize) break;
        }
    }

    return join(', ', $tmp);
}
\$\endgroup\$
2
\$\begingroup\$

My implementation would be pretty much the same as yours, but removing the foreach loop. It's not required as array_slice can do the job with less code.

function sizexpander($sizes)
{
     //These should be in order!
     $range = array("XS", "S", "M", "L", "XL", "XXL", "XXXL");

     //Split them into 2 segments, min / max + convert to upper
     $keys = explode("-",strtoupper($sizes));

     //Get the initial index of min
     $min = array_search($keys[0],$range);
     $max = array_search($keys[1],$range);

     //Slice the array and implode.
     return implode(",",array_slice($range,$min,($max - $min) + 1));
}

The main problem you had was that you had created a second function using string searching to find your segments. This has been replaced by explode which, as long as there is only ever 1 delimiter, then this would suffice.

Also, the while statement has been revoked, along with the string concatenation and been replaced with implode, which takes the found segments of the sizearray and implode them by a delimiter.

\$\endgroup\$
  • \$\begingroup\$ array_slice takes a starting index and a length, not a starting index and an end index. \$\endgroup\$ – sepp2k Mar 1 '11 at 11:12
  • \$\begingroup\$ Yea my bad, sorry about the confusion in your question aswell. \$\endgroup\$ – RobertPitt Mar 1 '11 at 11:30

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.