Php: how to lower complexity of method [closed]

Question

Closed. This question needs details or clarity. It is not currently accepting answers.

Want to improve this question? As written, this question is lacking some of the information it needs to be answered. If the author adds details in comments, consider editing them into the question. Once there's sufficient detail to answer, vote to reopen the question.

Closed 6 years ago.

Improve this question

I have this method rated B on Scrutinizer (direct link: https://scrutinizer-ci.com/g/sineverba/domotic-panel/inspections/76996c9f-543f-43b4-9475-c64fe810a278/code-structure/operation/App%5CHttp%5CControllers%5CApi%5CPublicIpController%3A%3Aupdate)

public function update()
{
    try {
        $data = array();
        $update = false;
        $string_previous_public_ip = null;
        $current_public_ip = $this->getGateway()->fetchPublicIp($this->getIp());
        $previous_public_ip = $this->getGateway()->getLastRecord();
        $data[ 'ip_address' ] = $current_public_ip;
        if (isset($previous_public_ip->ip_address)) {
            $string_previous_public_ip = $previous_public_ip->ip_address;
            $data[ 'id' ] = $previous_public_ip->id;
        }
        if ($current_public_ip != $string_previous_public_ip) {
            $update = $this->updateOrCreate($data);
        }
        return response()->json([
            'updated' => $update
        ], 200);
    } catch (ConnectionError $e) {
        // Will return error 500
    } catch (ServiceError $e) {
        // Will return error 500
    } catch (\Exception $e) {
        // Will return error 500
    }
    return response()->json([
        'updated' => $update
    ], 500);
}

Can I lower the cyclomatic complexity? I did move yet a if/else for the update (updateOrCreate method I think it is clear his work), but it is not sufficient.

Thank you!

Can you post a screenshot of the messaging from Scrutenizer in your question? — John Conde
– John Conde, Commented Mar 7, 2019 at 12:51
The current question title, which states your concerns about the code, is too general to be useful here. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How to get the best value out of Code Review: Asking Questions for guidance on writing good question titles. — Toby Speight
– Toby Speight, Commented Mar 7, 2019 at 13:29

Oh My Goodness · Accepted Answer · 2019-03-07 14:14:02Z

Your three catch blocks and two returns do the same thing as each other. You only need one of each.

$update is referenced outside of try, should be defined there too.

$current_public_ip is just another name for $data[ 'ip_address' ]. It's not any shorter or clearer and doesn't resemble its other name very closely.

$previous_public_ip isn't an IP address, it's an object that contains an IP address and other stuff. It would be clearer to simply name it $previous, which lets you drop the redundant string_ from $string_previous_public_ip. _public can go too.

public function update()
{
    $update = false;
    $status = 500; // default
    try {
        $data = array();
        $previous_ip = null;
        $data[ 'ip_address' ] = $this->getGateway()->fetchPublicIp($this->getIp());
        $previous = $this->getGateway()->getLastRecord();
        if ( isset($previous->ip_address) ) {
            $previous_ip = $previous->ip_address;
            $data[ 'id' ] = $previous->id;
        }
        if ( $data[ 'ip_address' ] != $previous_ip ) {
            $update = $this->updateOrCreate($data);
        }
        $status = 200;
    } catch ( \Exception $e ) {
        // return default status (500)
    }
    return response()->json([ 'updated' => $update ], $status);
}

Stack Exchange Network

Php: how to lower complexity of method [closed]

1 Answer 1

Hot Network Questions

Php: how to lower complexity of method [closed]

1 Answer 1

Related

Hot Network Questions