Buy

PUT: Killing Duplicated Code

PUT: Killing Duplicated Code

Our tests are passing, but we’re doing a bad job: I’ve created duplicated code in newAction and updateAction!

Let’s redeem ourselves! Create a new private function called handleRequest and copy the code into it that reads the request body and sets the data on the Programmer:

// src/KnpU/CodeBattle/Controller/Api/ProgrammerController.php
// ...

private function handleRequest(Request $request, Programmer $programmer)
{
    $data = json_decode($request->getContent(), true);

    $programmer->nickname = $data['nickname'];
    $programmer->avatarNumber = $data['avatarNumber'];
    $programmer->tagLine = $data['tagLine'];
    $programmer->userId = $this->findUserByUsername('weaverryan')->id;
}

Cool! Now we can just call this from newAction and updateAction:

// src/KnpU/CodeBattle/Controller/Api/ProgrammerController.php
// ...

public function newAction(Request $request)
{
    $programmer = new Programmer();
    $this->handleRequest($request, $programmer);
    $this->save($programmer);

    // ...
}

public function updateAction($nickname, Request $request)
{
    $programmer = $this->getProgrammerRepository()->findOneByNickname($nickname);

    if (!$programmer) {
        $this->throw404();
    }

    $this->handleRequest($request, $programmer);
    $this->save($programmer);

    // ...
}

Re-run the tests to see if we broke anything:

$ php vendir/bin/behat

Cool! I’m going to change how this code is written just a little bit so that it’s even more dynamic:

// src/KnpU/CodeBattle/Controller/Api/ProgrammerController.php
// ...

private function handleRequest(Request $request, Programmer $programmer)
{
    $data = json_decode($request->getContent(), true);

    if ($data === null) {
        throw new \Exception(sprintf('Invalid JSON: '.$request->getContent()));
    }

    // determine which properties should be changeable on this request
    $apiProperties = array('nickname', 'avatarNumber', 'tagLine');

    // update the properties
    foreach ($apiProperties as $property) {
        $val = isset($data[$property]) ? $data[$property] : null;
        $programmer->$property = $val;
    }

    $programmer->userId = $this->findUserByUsername('weaverryan')->id;
}

There’s nothing important in this change, but it’ll make some future changes easier to understand. If you’re using a form library or have a fancier ORM, you can probably do something like this with even less code than I have.

While we’re here, let’s throw a big exception if the client sends us invalid JSON.

Leave a comment!

  • 2016-10-06 Maksym Minenko

    Well, both Rest part are kind of heavy for me, frankly speaking... And they doesn't dwell on Silex *at all*.
    I guess I'll try "OAuth2 in 8 Steps", though.

  • 2016-10-06 Victor Bocharsky

    Hey Maksym,

    We have plans to create Silex tutorial, but have no any estimates on it yet. For now, you can check out a few course based on Silex: check "SIlex" tag on https://knpuniversity.com/cour... page.

    Cheers!

  • 2016-10-06 Maksym Minenko

    Any plans to actually create a tutorial on the Silex framework?