A puzzle about how to write more clean and understandability code

Question

private function getFeatureByParam ($classname, $module, $action) {
    $needFeature = array();
    foreach ($this->featureConf as $featureIndex => $feature) {
        if ($feature['classname'] == $classname && 
            $feature['module'] == $module && 
            $feature['action'] == $action) {
                $needFeature = $feature;
                break;
        }
    }
    return $needFeature;
}
private function getFeatureByParam ($classname, $module, $action) {
    foreach ($this->featureConf as $featureIndex => $feature) {
        if ($feature['classname'] == $classname && 
            $feature['module'] == $module && 
            $feature['action'] == $action) {
                return $feature;
        }
    }
    return array();
}

The first style of writing is more understandability.

The second style of writing is more clean and efficient.

But witch one is more better? (Or they are too bad....)

Answer 1

Personally, option 2.

In option 1, by breaking out of the loop, you have to look down until after the end to see what happens, just to see a return. In option 2, it's obvious what you want and when you get it. Although, I think the condition is a little complex.

There are, of course, other options:

private function getFeatureByParam ($classname, $module, $action) {
    foreach ($this->featureConf as $featureIndex => $feature) {
        if ($feature['classname'] != $classname) continue;
        if ($feature['module'] != $module) continue;
        if ($feature['action'] != $action) continue;

        return $feature;
    }
    return array();
}

private function getFeatureByParam ($classname, $module, $action) {
    foreach ($this->featureConf as $featureIndex => $feature) {
        if ( featureMatches( $feature, $classname, $module, $action ) ) {
            return $feature;
        }
    }
    return array();
}

I'm inclined to option 4 :)