Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I'm trying to reduce the complexity of some methods, and I'm not exactly sure what approach to take. I'm currently building a PHP wrapper for a REST API, and the main problematic class is here:

In short, since data retrieved from the API is not always consistent, I have to do this:

$portrait = new Portrait();
$portrait->setXCoordinate(isset($apiData['portrait']['x']) ? $apiData['portrait']['x'] : null)
    ->setYCoordinate(isset($apiData['portrait']['y']) ? $apiData['portrait']['y'] : null)
    ->setWidth(isset($apiData['portrait']['w']) ? $apiData['portrait']['w'] : null)
    ->setHeight(isset($apiData['portrait']['h']) ? $apiData['portrait']['h'] : null)
    ->setOffset(isset($apiData['portrait']['offset']) ? $apiData['portrait']['offset'] : null)
    ->setUrl(isset($apiData['portrait']['url']) ? $apiData['portrait']['url'] : null);
return $portrait;

What would be a better way of handling the problem, so that the method complexity goes down? I suppose that I can try and catch an "undefined index exception", but I kind of wanted to see if there's something a bit more elegant that I can use.

share|improve this question
up vote 4 down vote accepted

You could remove some duplication with a getPortraitValue function:

function getPortraitValue($apiData, $key) {
    $portrait = $apiData['portrait'];
    if (isset($portrait[$key])) {
        return $portrait[$key];
    return null;

$portrait = new Portrait();
$portrait->setXCoordinate(getPortraitValue($apiData, 'x')
    ->setYCoordinate(getPortraitValue($apiData, 'y'))
    ->setWidth(getPortraitValue($apiData, 'w'))
    ->setHeight(getPortraitValue($apiData, 'h'))
    ->setOffset(getPortraitValue($apiData, 'offset')
    ->setUrl(getPortraitValue($apiData, 'url');
return $portrait;
share|improve this answer
Yep, was thinking the same thing at one point, but this solution is essentially just a wrapper for the inline if. I say this because it is in fact a tad incorrect - if $apiData['portrait'][$key] does not exist, you'll still get a "Undefined index" exception, so instead you have to do: if (isset($apiData['portrait'][$key])) return $apiData['portrait'][$key]; – Petre Pătraşc Feb 28 '14 at 13:51
@PetrePătraşc: Thank you, I've fixed that. – palacsint Feb 28 '14 at 14:05
I'll try running some code analysis on that and checking if the lack of ternary does bring it down. – Petre Pătraşc Feb 28 '14 at 14:07
Yep, you were right! Method has been improved!… Accepting your answer, as it is the right solution. – Petre Pătraşc Feb 28 '14 at 14:29
@PetrePătraşc: Good to hear that, thanks :) – palacsint Feb 28 '14 at 14:32

Your Answer


By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.