Phpinsights: multiple catches increase cyclomatic complexity calculation

Created on 12 Jun 2019  路  6Comments  路  Source: nunomaduro/phpinsights

| Q | A
| ---------------- | -----
| Bug report? | yes
| Feature request? | no
| Library version | 1.6.0

In a Controller, I have 6 methods that have the same structures

public function doSomething($a, $b)
    {
        try {
            return $this->http_client->post('do-thing', [
                'headers' => $header,
                'json' => [
                     // parameters here
                ]
            ]);
        } catch (ClientException $e) {
           // report to sentry.io
            if (app()->bound('sentry')) {
                app('sentry')->captureException($e);
            }

            return response()->json(['message' => 'The given data was invalid'], 422);
        } catch (ServerException $e) {
            if (app()->bound('sentry')) {
                app('sentry')->captureException($e);
            }

            return response()->json(['message' => 'Server Error'], 500);
        } catch (\Throwable $e) {
            if (app()->bound('sentry')) {
                app('sentry')->captureException($e);
            }

            return response()->json(['message' => 'Unexpected Error'], 500);
        }
    }

If I have only 2 or less of doSomething, I dont get the cyclomatic complexity complains. When I have 6 of doSomething, I have 38 cyclomatic complexity level. Well, it doesnt look like a level 38 to me.

question

Most helpful comment

@olivernybroe thank you for your kind assistance.

All 6 comments

Could you explain in a little more detail what you mean with the same structure? Do you mean that the whole catch part is completely identical in all of the methods?

The complexity of a class is increased by the amount of statements in it. So taking just the statements in the method you have here, the complexity is increased by 6.

@olivernybroe I have 6 functions namely doSomething1, doSomething2, .. , doSomething6

Each of them will try to call post to external API and that's it. There is no if/else, loop in the try part. Each of them will have the exact same catch part.

Without seeing actual code I cannot help to be honest.

Complexity increased with the amount of statements in your class, as it is getting more complex per statement added.

As far as I can see in the code you showed in the top and from what you have told, this is not a bug.

Yep, looking at your code this is not a bug.

As I said complexity is increased when the amount of statements increases. In this class there are a lot fo statements and they could easily be refactored to fewer statements.

I can give you a couple of hints to reduce the complexity.
https://gist.github.com/olivernybroe/2623c29769282e6946f76542ec6df55f

Then you could also consider moving

       if (app()->bound('sentry')) {
            app('sentry')->captureException($exception);
        }

into your handler class instead.

@olivernybroe thank you for your kind assistance.

Was this page helpful?
0 / 5 - 0 ratings