How difficult is to refactoring a goto ?

A few year ago I need to use AWS-SDK library for php. I find that is incompatible with xdebug because a "segmentation fault" error can occurs. The error is generated in TraceMidleware::str by the use of var_dump. The xdebug change output of var_dump. As conclusion is difficult to debug your project if you have AWS-SDK library. As quick solution, the var_dump can be replaced with print_r or another similar function. The code that generate error:

    ob_start();
    var_dump($value);
    return ob_get_clean();
            

Enough with this error, and go to "goto". I find the goto directive in src/ClientResolver.php. The code that use this directive in general is difficult to read and understand. Explanation about why not to use "goto" can be found in Edsger Dijkstra's letter "Go To Statement Considered Harmful".

First I download the library install dependency and execute tests. The test are failing and if you have xdebug enabled, then even more tests are failing. In the library help page I don't find information about how to execute tests.

    git clone https://github.com/aws/aws-sdk-php.git aws-sdk
    composer.phar install
    phpunit
            

For my luck all tests for ClientResolver are green if xdebug is disabled (2 failing with xdebug enabled) so I go to replace "goto". Initial code: with goto incorporated:

    public function resolve(array $args, HandlerList $list)
    {
        $args['config'] = [];
        foreach ($this->argDefinitions as $key => $a) {
            // Add defaults, validate required values, and skip if not set.
            if (!isset($args[$key])) {
                if (isset($a['default'])) {
                    // Merge defaults in when not present.
                    if (is_callable($a['default'])
                        && (
                            is_array($a['default'])
                            || $a['default'] instanceof \Closure
                        )
                    ) {
                        $args[$key] = $a['default']($args);
                    } else {
                        $args[$key] = $a['default'];
                    }
                } elseif (empty($a['required'])) {
                    continue;
                } else {
                    $this->throwRequired($args);
                }
            }
            // Validate the types against the provided value.
            foreach ($a['valid'] as $check) {
                if (isset(self::$typeMap[$check])) {
                    $fn = self::$typeMap[$check];
                    if ($fn($args[$key])) {
                        goto is_valid;
                    }
                } elseif ($args[$key] instanceof $check) {
                    goto is_valid;
                }
            }
            $this->invalidType($key, $args[$key]);
            // Apply the value
            is_valid:
            if (isset($a['fn'])) {
                $a['fn']($args[$key], $args, $list);
            }
            if ($a['type'] === 'config') {
                $args['config'][$key] = $args[$key];
            }
        }
        return $args;
    }
            

Simple solution, is to move the code from is_valid: in a separate function and create a new variable $isValid, for prevent to execute the function invalidType if the type is not valid. The code after remove goto:

public function resolve(array $args, HandlerList $list)
    {
        $args['config'] = [];
        foreach ($this->argDefinitions as $key => $a) {
            // Add defaults, validate required values, and skip if not set.
            if (!isset($args[$key])) {
                if (isset($a['default'])) {
                    // Merge defaults in when not present.
                    if ($this->isFunction($a)) {
                        $args[$key] = $a['default']($args);
                    } else {
                        $args[$key] = $a['default'];
                    }
                } elseif (empty($a['required'])) {
                    continue;
                } else {
                    $this->throwRequired($args);
                }
            }
            $isValid = false; // prevent to generate error if type is valid
            // Validate the types against the provided value.
            foreach ($a['valid'] as $check) {
                if (isset(self::$typeMap[$check])) {
                    $fn = self::$typeMap[$check];
                    if ($fn($args[$key])) {
                        // apply old goto code and mark as valid
                        $args = $this->applyProvidedValue($args, $list, $a, $key);
                        $isValid = true;
                        break;
                    }
                } elseif ($args[$key] instanceof $check) {
                    // apply old goto code and mark as valid
                    $args = $this->applyProvidedValue($args, $list, $a, $key);
                    $isValid = true;
                    break;
                }
            }

            // execute only if we can validate
            if (!$isValid) {
                $this->invalidType($key, $args[$key]);
            }
        }

        return $args;
    }
        

The two extracted function:

    // extract a function from if condition because was to large
    private function isFunction($a)
    {
        return is_callable($a['default'])
            && (
                is_array($a['default'])
                || $a['default'] instanceof \Closure
            );
    }

    // the same content that was in is_valid:
    private function applyProvidedValue(array $args, HandlerList $list, &$a, $key)
    {
        if (isset($a['fn'])) {
            $a['fn']($args[$key], $args, $list);
        }

        if ($a['type'] === 'config') {
            $args['config'][$key] = $args[$key];
        }
        return $args;
    }
        

* The version of aws/aws-sdk-php that I use https://github.com/aws/aws-sdk-php/tree/3.110.1