commerceguys/zone

Bug in PostalCodeHelper::buildList

Closed this issue · 2 comments

If you try:
PostalCodeHelper::buildList('5:7,10:12')
you'll get something like:
[6,7,10,11,12]
and "5" will be missing because of the mix of foreach, array_merge and unset($postalCodeList[$index]) .

        $postalCodeList = explode(',', $postalCodes);
        foreach ($postalCodeList as $index => &$postalCodeItem) {
            $postalCodeItem = trim($postalCodeItem);
            if (strpos($postalCodeItem, ':') !== false) {
                $postalCodeRange = explode(':', $postalCodeItem);
                if (is_numeric($postalCodeRange[0]) && is_numeric($postalCodeRange[1])) {
                    $postalCodeRange = range($postalCodeRange[0], $postalCodeRange[1]);
                    $postalCodeList = array_merge($postalCodeList, $postalCodeRange);
                }
                unset($postalCodeList[$index]);
            }
        }

Here is my solution:

   /**
     * Builds a list of postal codes from the provided string.
     *
     * Expands any ranges into full values (e.g. "1:3" becomes "1, 2, 3").
     *
     * @param string $postalCodes The postal codes.
     *
     * @return array The list of postal codes.
     */
    protected static function buildList($postalCodes)
    {
        $resultList = [];
        $postalCodeList = explode(',', $postalCodes);
        foreach ($postalCodeList as $index => $postalCodeItem) {
            $postalCodeItem = trim($postalCodeItem);
            if (strpos($postalCodeItem, ':') !== false) {
                $postalCodeRange = explode(':', $postalCodeItem);
                if (is_numeric($postalCodeRange[0]) && is_numeric($postalCodeRange[1])) {
                    $postalCodeRange = range($postalCodeRange[0], $postalCodeRange[1]);
                    $resultList = array_merge($resultList, $postalCodeRange);
                }
                continue;
            }

            $resultList[] = $postalCodeItem;
        }

        return $resultList;
    }

The helper got moved to commerceguys/addressing, opened a matching issue there so that I can remember to address this. Thanks for reporting.

Fixed upstream. Thanks for the report!