Leaderboard

Rule #3 is about avoiding something like this: function blah($list){ foreach ($list as $a){ if ($a['flag']){ foreach ($a['list'] as $b){ if ($b['flag']){ //something } else if ($b['flag2']){ foreach ($b['list'] as $c){ if ($c['x']){ //something } else { //other thing } } } } } } } It's hard to read, particularly when the //something's are longer than one line and it wastes horizontal space. It'd be better to pull chunks out into other functions, for example: function blah($list){ foreach ($list as $a){ if ($a['flag']){ processFlagList($a); } } } function processFlaglist($list){ foreach ($list as $b){ if ($b['flag']){ //something } else if ($b['flag2']){ processFlag2($b); } } } function processFlag2List($list){ foreach ($b['list'] as $c){ if ($c['x']){ //something } else { //other thing } } } Much easier to follow, especially if you only want a high-level overview of the code and don't need the details. This can be valuable in some big if/else chain like described above. Imagine you were just quickly trying to get an idea of what some function does and you see this: function blah($list){ foreach ($list as $a){ if ($a['flag1']){ //something 20 lines long } else if ($a['flag2']){ //something else 20 lines long } else if ($a['flag3']){ //yet another thing 30 lines long } else { //and finally the last thing 8 lines long } } } Remember, imagine all those //something's are big blocks of code that prevent you from seeing the whole tree like you can here. That'd be a whole lot of code to read through, figure out what it does and keep track of it all. What if instead you saw this: function blah($list){ foreach ($list as $a){ if ($a['flag1']){ sendToProcessor($a); } else if ($a['flag2']){ markCompleted($a); } else if ($a['flag3']){ notifyTheCEO($a); } else { setEverythingOnFire(); } } } Much easier and the names of the function let you know what's happening without having to dig deeper. If you decide you need to know exactly how the code sets everything on fire you can easily* go find out, but if you don't care then that code isn't cluttering up your view. * With a good IDE going and finding out what a function does is easy. In PHPStorm for example I could just CTRL+Click on (or CTRL+B with the text cursor in) the function name and it'll take me to it. Without a good IDE it might be harder.

I pretty much agree with kicken, but feel the need to throw in my own two cents. First off, tip #3 is something everyone who codes should live by. To add to it, don't string together a million conditionals - I'm currently dealing with a code base that has a 180-line if-elseif-else tree and it makes me wanna barf. For what is supposed to be a conditional logic operation, this is shockingly devoid of both condition and logic. Personally, I think any sort of hard and fast numerical limit on function/method/class width or length is kinda manufactured. Use judgment and good formatting. For the width, break across lines when it becomes hard to read. For instance, there's nothing inherently wrong with this code: if(empty($_POST['firstname']) || empty($_POST['lastname'])){ // ... } Plenty easy to read, but if you've got more validation to do, this becomes a nightmare and should be broken, like so: if( empty($_POST['firstname']) || empty($_POST['lastname']) || empty($_POST['middlename']) || empty($_POST['fieldicareabout']) || empty($_POST['anotherimportantfield']) || empty($_POST['myrefridgerator']) ){ // ... } Granted, this board's indenting is far larger than mine in VSCode, but hopefully you get the point. As far as length, methods should do one thing, and objects should handle one situation. What you've got now is what's called a god object in OOP, and it's - as you're finding - difficult to reason about and maintain. On the other hand, the issue you can run into with one function per file is that it can become a mess of code relationship that's hard to follow if you're not extremely diligent. It also absolutely requires a full test suite because chances are you'll be including the same file in multiple places, and this can lead to hard to find bugs when you forget you use a function somewhere in a different part of the code. And finally, just to beat the horse some more, format everything appropriately. I've spent untold hours at work trying to parse SQL queries because the original author wrote them out in one long line instead of using line breaks and indenting to make it make sense. Obviously PHP isn't Python or Ruby in that indentation doesn't actually affect the code, but there's a reason the authors of some programming languages chose to use indentation instead of brackets or curly braces - it makes the code easier to read, and illuminates the intention of the functionality. Wait - I lied; one more thing (it's late but I'm not sleepy and my mind is going, sorry). Absolutely use descriptive and appropriate method and variable names, but document as well. Just because your method is called `applySalesTax` doesn't mean I feel like parsing the entire method to figure out how or why you're applying sales tax. For instance you can assume that people know that sales tax is different across states, but if you don't charge sales tax in North Carolina as a business decision or due to a legal loophole, put that in the docblock.