I am declaring a personal crusade to improve Drupal’s “Developer Experience,” which I hereby abbreviate as “DX.”
User experience can be defined as “the overall experience and satisfaction a user has when using a product or system.” A product’s UX affects a user’s interest, ability, and enjoyment when using a product. If a product’s UX sucks, people will not use it if they have a choice or hate using it if they do not.
Well, guess what? Software engineers are people, too. When creating a software product that is intended to be used by other developers, the UX includes the experience of those developers as they write code for the product. But software engineers aren’t exactly like normal people and the kinds of issues developers face are very different from the issues the end users of the product face, so it seems logical to me to talk about DX distinctly from UX just to keep the target audience straight.
My first attempt at a definition for DX is “the overall experience, satisfaction, and efficiency a software developer has when using a software development platform.” I’m sure a better definition is possible.
The Drupal community has not historically been great at UX design (mostly we’re just a bunch of hackers) but recently has focused its enormous talents in this direction and started to make great strides. Somewhat surprisingly, we have also not historically been great at DX design. Drupal is quite powerful and flexible but unless you climb a fairly steep learning curve it can be hard to develop for. I have personally spoken with a number of web developers who have evaluated Drupal and found it too annoying, too unlike everything they have done before, and sometimes just too “backwards” for them to want to use it.
There are many, many ways to improve Drupal’s Developer Experience. Some are trivial, some are complex, and many are in between. My plan is for “The DX Files” to be a series of articles describing what I consider to be poor Drupal DX and suggesting solutions.
Since I’ve already rambled on a bit, I’ll start with a very simple DX issue for today: string literals vs. defined constants. Consider this completely typical example from Drupal’s user.module:
<?php
function user_register_access() {
return user_is_anonymous() && variable_get('user_regsiter', 1);
}
?>This function determines if a particular visitor has permission to register for an account. It says the user must not yet be logged in and that the site-wide “new user accounts are allowed” (user_register) setting is enabled. The variable_get() function takes a default value as its second argument which, in this case, is 1 for “enabled.”
The example uses a string literal ‘user_register’ instead of the much more accepted approach of defined constants. Why would a defined constant be better? All the standard reasons: more readable code, the ability to change the constant in one place instead of many, detection of typos at compile time, auto-completion in IDEs, etc.
Case in point: did you notice that user_regsiter is mis-typed? (Note: I introduced the typo for this example; the real code is correct.) The program still “works” but contains a security flaw: If the administrator sets the “user_register” setting to disabled, this code will not see it because it is looking for “user_regsiter” and as a result will use the default value of “enabled.” Oops.
If the user module used a defined constant, the bug could not occur:
<?php
define('USER_REGISTER', 'user_register');
function user_register_access() {
return user_is_anonymous() && variable_get(USER_REGSITER, 1);
}
?>Now the code will simply fail to compile and the typo will be discovered immediately.
I actually believe there are two other flaws in user_register_access(), one DX-related and one security-related. Will you be the first to point them out?
UPDATE on May 19, 2008: I’ve submitted at patch to replace all Drupal “variable” names with defined constants; see http://drupal.org/node/260226.
To happier coding!
Comments
Good call, Barry! I'm a big
Good call, Barry! I'm a big fan of defined constants for exactly the reasons you've stated (IDE support means something to me these days too even).
+1
Barry, what you point out is
Barry, what you point out is only half the problem there.
My experience with 'define' in PHP is much less satisfactory than it is with #define in C++. For one, it creates an extra step for the developer, and it's a step that adds a fair bit of code.
But really, that whole variable_get() call is flawed; you put the default in at variable_get() time, which is much worse than making a typo in the name of the string.
Strings are no less searchable than defines (at least, 'user_register' vs USER_REGISTER is going to be pretty much equivalent in my editor's search), and you do gain the typo checking, kind of. (Note: only if you have E_NOTICE on, which Drupal now does but this is new as of Drupal 6).
Of course, you could get the exact same effect by having something that defines the default, and have variable_get() fail if it fails to find both a default AND a set value. And you would gain consistency in the variable_get() default values, which is a much worse problem than the occasional typo in the variable_get string, since changing the defaults actually is kind of a pain.
How about the use of 1 as
How about the use of 1 as opposed to a USER_REGISTRATION_ENABLED constant?
Typo correction: You must
Typo correction:
You must mean define(), not defined().
Earl: I *absolutely* agree
Earl: I *absolutely* agree with you that having to specify the default to variable_get() is a horrible design; that's the other UX flaw I referred to in the penultimate paragraph. I would support a patch to change the variable system so that variables are declared, documented, and given a default in a module hook (yet another registry!). There are several issues in the queue about this; one of them is http://drupal.org/node/145164.
greggles: A USER_REGISTRATION_ENABLED constant wouldn't be a bad idea, but that was not one of the flaws I had in mind. In general, I think using boolean TRUE/FALSE or 0/1 values is fine; we don't need a zillion define()s for TRUE/FALSE. But that's just my opinion.
Benjamin: Thanks for the correction; fixed.
The security flaw I referred
The security flaw I referred to: If the USER_REGISTRATION variable has never been set, the variable_get() call defaults to returning "enabled" so users will be able to register for accounts. I've seen plenty of bugs in Drupal where code assumes that a variable has been set by having the settings form submitted and ends up with unintentional behavior if it has not.
The current behavior reflects Drupal's history as originally being developed for open blogging sites but may not be well-suited to its growing use as a platform for a wider variety of sites.
I’ve submitted at patch to
I’ve submitted at patch to replace all Drupal “variable” names with defined constants; see http://drupal.org/node/260226.
Did you test what you're
Did you test what you're writing?
PHP will (by default) interpret any unknown literal as a string. Thus your claim that "the code will simply fail to compile and the typo will be discovered immediately" is completely wrong.
Yes, I implemented and
Yes, I implemented and tested these changes; see http://drupal.org/node/260226.
Drupal 6 has E_ALL warnings enabled. So, true, the compile will not actually fail, but the warning will be prominently displayed and noticed.
<?phperror_reporting(E_ALL);
define("A", "a");
define("A", "b");
print C;
?>
Notice: Use of undefined constant C - assumed 'C' in Command line code on line 1Notice: Constant A already defined in Command line code on line 1
C
I would support the idea of
I would support the idea of having a hook_variables where the module developer declare, document and define default values for the variables. As you say, this hook would be yet another 'registry hook', like would be hook_blocks (http://drupal.org/node/257032), and others, ...
Having a column in the variables table to store the module that provides the variable would be very useful, the system automatically can delete the variables when a module is uninstalled and modules won't need to worry about this.
Post new comment