Argument order of "Api::api" method is not correct

[dereuromark]

The order as per clean code should not have optional values prior to required ones:

    public function api(
        $method = self::REQUEST_GET,
        $url,
        $data = array(),
        $return_as_json = false,
        $is_file = false,
        $debug = false
    )

Instead if should be

    public function api(
        $url,
        $method = self::REQUEST_GET,
        $data = array(),
        $return_as_json = false,
        $is_file = false,
        $debug = false
    )

So you can actually do

    ->api($url)

and the others are actually optional.

[aik099]

Yeah, I know. This is the problem with several other methods as well. PhpStorm always complains about it to me.

Unfortunately we can't change argument order because that would be a BC break.

[aik099]

What we can do is to remove default value for $method argument. That sound like BC break, but in fact it is not, because when manually calling Api::api method you have to specify $method all the time and trick like specifying null instead to get default value used won't work currently.

[dereuromark]

I agree. And maybe in a future major bump we can fix it.

[aik099]

So, if you'd like you can send PR that would remove default value for optional method arguments, that come (in method declaration) before non-optional arguments. There are several such methods for sure.

[aik099]

And maybe in a future major bump we can fix it.

I don't think, that changing argument order is a way to go, because in API calls the result changes greatly based on the request method (get/post/etc) so specifying it first and then url makes sense actually.

[aik099]

@dereuromark , should I expect PR for review soon?