10

I have two particular cases where I disagree with a coworker, whether constants should be used or not.

We use a homemade framework working roughly like Symfony 1.x.

  1. Initial code was, in a routing PHP config file for routing, like this one:

    $router->map('/some_url', array('module' => 'some_module', 'action' => 'some_action'));
    $router->map('/some_other_url', array('module' => 'some_module', 'action' => 'some_action'));
    // etc.
    

    The coworker changed it to:

    $router->map('/some_url', array(MODULE => 'some_module', ACTION => 'some_action'));
    $router->map('/some_other_url', array(MODULE => 'some_module', ACTION => 'some_action'));
    
    // + in constants.php file:
    define('MODULE', 'module');
    define('ACTION', 'action');
    

    IMO this is constant overuse: if the concept of "module" or "action" is ever renamed, it would have to be renamed in the entire code, either written as a string or a constant. Plus, the defined constant names above have no specific meaning, favoring naming collisions/confusions.

  2. Initial code example:

    if (isset($_SESSION['unid']) && isset($_SESSION['login'])) { ... }
    

    Modified by the coworker:

    if (isset($_SESSION[UNID]) && isset($_SESSION[LOGIN])) { ... }
    
    // + in a constants.php file:
    define('UNID', 'unid');
    define('LOGIN', 'login');
    

    In our application, those session vars names unid and login are clearly unlikely to change. Nonetheless, if declaring constants was really a good practice here, I would suggest at least more precise names, for example FIELDNAME_UNID and FIELDNAME_LOGIN...

Is introducing those constants really relevant (that is, naming should just be improved), or (as I guess) completely useless ?

Thanks.

EDIT

After a few months, here are a few (incredible) lines from the constants.php file. I definitely find this a completely useless mess, similar to this DailyWTF post. Too many constants kills constants.

define('POST', 'POST');
define('GET', 'GET');

define('PROJECT', 'project');
define('APPLICATION', 'application');
define('MODULE', 'module');
define('ACTION', 'action');
define('ID', 'id');
define('SLUG', 'slug');
define('CONTROLLER', 'controller');
define('CONTENT', 'content');
define('AJAX', 'ajax');
define('EXECUTE', 'execute');
define('FORMAT', 'format');
define('BASE_HREF_CONSTANT', 'basehref');
define('UNID', 'unid');
define('USERNAME', 'username');
define('PASSWORD', 'password');
define('TEMPLATE', 'templates');
define('UNSECURE', 'unsecure');
define('MODE', 'mode');
define('MESSAGE', 'message');
define('TEMPORARY_SESSION', 'temporary_session');
define('ERRORMESSAGE', 'errormessage');
define('START_FROM', 'startfrom');
define('COUNT', 'count');

// and so on.
Maxime Pacary
  • 22,336
  • 11
  • 85
  • 113
  • Related: http://stackoverflow.com/questions/4234129/should-i-use-constants-instead-of-strings-even-if-the-strings-are-only-ever-used – Maxime Pacary Jun 28 '11 at 23:37
  • 1
    Not exactly related (concerns hardcoded numbers) but worth a read: http://stackoverflow.com/questions/47882/what-is-a-magic-number-and-why-is-it-bad – James P. Jun 28 '11 at 23:43
  • doesn't it always end up at personal preference. –  Jun 29 '11 at 00:07
  • Yes, but sometimes it leads to poor design. I said "matter of personal preference" below, because in those cases, choosing any of the two options do not seems an obvious bad practice. – Maxime Pacary Jun 29 '11 at 00:12

3 Answers3

10

Advantages

  • consequences of misspelling constant should trigger an E_NOTICE 'Use of undefined constant', whereas misspelling the string literal would not offer such an early warning.
  • if followed to its logical conclusion, any remaining string literals in the code should be natural language, and thus the task of identifying strings to be wrapped in an internationalization translation layer is made a little easier.

Disadvantages

  • requires you define all constants whether you need them or not. Not likely to be your performance bottleneck unless you're defining thousands of them though!
Paul Dixon
  • 295,876
  • 54
  • 310
  • 348
7

There is a valid argument for using constants like that.

If you accidentally do something like:

$router->map('/some_url', array('moduel' => 'some_module', 'action' => 'some_action'));

it will fail in some undefined way (note the misspelled "moduel").

If you make a spelling mistake or typo when a constant is involved, PHP emits a Notice, and you catch it right away.

How often that actually saves you is a matter for debate. Personally, I usually don't think it's worth the trouble.

timdev
  • 61,857
  • 6
  • 82
  • 92
  • It's an argument. However in this case the *map()* method should check required data, and in this case would complain for the missing 'module' info. – Maxime Pacary Jun 28 '11 at 23:39
  • Additionally (even if it's unlikely to happen), if your spelling mistake ends to an already *other* defined constant, the PHP notice won't show up :) – Maxime Pacary Jun 28 '11 at 23:42
  • @FrostyZ - I almost said something like that, but decided against it. In that particular example, it's likely that map() would default to whatever the current module was, in the absence of a 'module'-keyed parameter. That could fail quite silently. – timdev Jun 28 '11 at 23:43
  • @FrontyZ - Regarding your second comment, yeah, then you're back at square one completely. Like i said in my answer, doesn't seem worth the trouble. – timdev Jun 28 '11 at 23:44
  • 1
    After a few months, reading this again... Replying to timdev three comments above: I could argue that the "parameters checking" could simply check for a list of "supported parameters" (required/optional). Then, in case of typing error like 'moduel', some message can be shown to the developer (because this parameter name is not among the function's "supported parameters"). – Maxime Pacary Sep 30 '11 at 13:55
3

And main reason is IDE autocompletion, of course.
With using constants you will not even need to write full name of constant (IDE will help you in this) and you will be absolutely sure that you have no typos.

It's not overuse. For example, I'm trying to avoid using strings in code.

OZ_
  • 12,492
  • 7
  • 50
  • 68
  • Autocompletion is an argument. However: either you clutter your global namespace with vague constant names by doing many `define('CONSTANT', 'constant');`, then autocompletion may be counter-productive (may propose a lot of values + wrong values among proposals + you cannot be 100% sure that when misspelling, the resulting constant name is not *already* defined + naming collisions problem...). Either you properly name your constants, like `Router::CODE_ACTION` or `ModelUser::FIELD_LOGIN` and it would be interesting to precisely compare readability, typing quickness, error prevention, etc. – Maxime Pacary Jun 29 '11 at 08:44
  • ... avoiding "magic numbers" and "magic strings" is obviously a good thing, but as any rule, it may have exceptions. – Maxime Pacary Jun 29 '11 at 08:46
  • @Frosty Z, nowadays I don't use global constants, only class-constants. And yes, of course exceptions for this "rule" should exist, or code will be too long :) – OZ_ Jun 29 '11 at 09:26