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 :)

• bloody numen: Wow,i had never thought the writing style of option 4. It is more clean and packaging the matching operation.Thank you very much~
• mseancole: My only issue with this answer is your braceless syntax. But that is more of a point of preference.
• Glenn Rogers: I only have braceless when it's a precondition-style break, continue, or return. Anything else gets one - it's too dangerous not to.