Firehed's Blog

What good are standards if you don't enforce them?

During our early phases with WePay, we’re considering development using a framework such as CakePHP. I’m not fundamentally opposed to using frameworks, but it’s important to understand the tradeoffs. They’re a good way to get up and running quickly, but you’re forced to use certain conventions that may or may not be good - and due to the way that most work, you’re usually making a massive tradeoff from performance to get up and running quickly (I think it should be obvious to developers why every page load running an EXPLAINDESCRIBE query that’s cached for no more than thirty seconds should be somewhat of a performance concern). While I may tend to err on the side of premature optimization, there are still some things that just violate best practices. Web apps like PHPMyAdmin using EXPLAINDESCRIBE on every page load makes perfect sense, that tends not to be the case on a web app that you’d like to see scale out to millions of users eventually (Hi, Twitter!).

(Yes, I know that there are caching adjustments you can make in order to negate this once your main app development is done. Fixing performance can be relatively easy if you know what you’re doing, or at least know where to look. But is it wrong to be nervous about variables being that dynamic? I think not, though it’s certainly a matter of opinion.)

Anyways, back on topic - one of the main points that Bill brought up in his arguments for using a framework is that it forces us to follow certain coding conventions and standards which should make maintainability less of an issue down the road. A very valid point, and certainly a good way to do things, if the code supports it. However, code conventions and standards aren’t exactly PHP’s strongest point: a very large amount of its core code-base (especially the earlier stuff) is the very definition of inconsistency and esoteric naming conventions. Is it array_search($needle, array $haystack) or array_search(array $haystack, $needle)? What about array_keys($search_value, array $array) vs array_keys(array $array, $search_value)? (The former, then the latter). How about the approximately 183 different string manipulation functions, each of which following some sort of C naming convention under the assumption that a function name that made sense would call sudo rm -rf /.

Don’t get me wrong - it’s still a very powerful language for what I do on a daily basis, and any real limitations I’ve encountered have been on my end rather than PHP’s. It just means that even after working with it for nearly a decade, I still have to hit the documentation much more often than seems natural (granted, if TextMate had a more powerful syntax completion engine (CodeSense, IntelliSense - whatever you want to call it), this would largely be a non-issue; I hear that’s coming in TextMate 2.0 but that’s beside the point). Thankfully, most of the newer built-in functionality follows some sort of logical naming conventions.

Back to frameworks. It makes sense on the surface, right? Define a model as class somename extends AppModel in models/somename.php, define your controller methods as class SomenamesController extends AppController and function action() in controllers/somenames_controller.php, and your views as viewname.ctp in views/somename/, and let the built-in routing functions figure out what gets called where, when, and how. The issue is that while you should follow naming conventions, you don’t really have to in order for things to work properly.

Wait, what?

Why would one even describe naming conventions if you don’t have to follow them? “Hi, there. I’d really prefer if you did things my way. It doesn’t take you any extra effort whatsoever. But if you really feel the need to go off and do it your own way, I don’t mind wasting a ton of time trying to figure out what the hell you were thinking and do it your way.”

Yeah, that doesn’t seem to make sense to me either. I suppose that really is the PHP way - let people know how they should do things, but try to recover from things if the programmer needs to defy conventions. It’s just too damn forgiving. Coders are used to fixing mistakes - trigger a program-aborting error and make them fix the mistake, rather than issue a (hidden by default, in most installations) warning and continue on your merry way.

I’d like to briefly examine CakePHP’s Controller::set (cake/libs/controller/controller.php:652) method here, as it so clearly illustrates the issue.

function set($one, $two = null) {
$data = array();

if (is_array($one)) {
if (is_array($two)) {
$data = array_combine($one, $two);
} else {
$data = $one;
}
} else {
$data = array($one => $two);
}

foreach ($data as $name => $value) {
if ($name === 'title') {
$this->pageTitle = $value;
} else {
if ($two === null && is_array($one)) {
$this->viewVars[Inflector::variable($name)] = $value;
} else {
$this->viewVars[$name] = $value;
}
}
}
}

Shall we step through things? A typical function, accepting two arguments, the latter being optional. It’s used to make variables available to views without excessive futzing with scope and digging into object properties (by use of the extract function). Makes enough sense. It’s basically just building an associative array. My issue here is that its excessive flexibility on inputs is unnecessary, adds complexity, and reduces the need to follow any one set of conventions. In fact, it allows three: set($key, $value), set(array $keys, array $values), and set(array $associativeArray). Each are valid (less so on the second of the three, IMO), but the mere fact that three are allowed is my major concern. Either of these approaches seems far better:

function set($key, $value) {
$this->viewVars[$key] = $value;
}

or

function set(array $associativeArray) {
foreach ($associativeArray as $key => $value) {
$this->viewVars[$key] = $value;
}
// Alternately: $this->viewVars = array_merge($this->viewVars, $associativeArray);
}

Simple, no? The exception with a key of “title” should be processed at runtime by View::_render() (where this data is eventually used) if it deserves an exception at all. Either option forces you to use a SINGLE convention throughout your code, rather than picking from whichever seems the most appropriate of three at any given time.

So, that’s my rant for the day. If you’re going for consistency, be consistent and enforce it. If you’re not going to enforce it, then don’t even claim to require it. It’s one thing if you need to maintain some level of backwards compatibility (though this should ideally be done via wrapper/abstraction methods), but that shouldn’t be the case for frameworks which are built so for sites with a 0-byte codebase. Forcing MVC for site operation is a good thing when appropriate, and I intend to employ a similar approach to what CakePHP has implemented in my own code (albeit through slightly different means).

Writing code in a way that you don’t need to reinvent the wheel is a good thing. But what engineer in his/her right mind would provide three different sets of mounting points for that wheel just because some companies want to use agreed-upon metric bolts while one wants to use a coarse-threaded imperial bolt and one other wants to use a fine-thread of the same diameter. Yes, you can do it - but it will wreak havoc on your structural stability.

And now that the requisite car analogy is out of the way, I’m done. Go outside already, it’s a beautiful day.

Edit: Realized I mistakenly ranted about using EXPLAIN when it should have been DESCRIBE up at the top of the post. Serves me right for not proofreading.