Home / Comment permalink

The DX Files: Defined constants as API arguments

This is part two of my series, The DX Files: Improving Drupal Developer Experience.

Many Drupal APIs accept a boolean argument (TRUE or FALSE) to determine some behavior. I believe that practice should be banned in all but exceptional cases, instead using a defined constant with a descriptive name.

Here is a perfect example from Drupal core:

<span style="color: #000000"><span style="color: #0000BB">&lt;?php<br />&nbsp;&nbsp;&nbsp; $output </span><span style="color: #007700">= </span><span style="color: #0000BB">node_view</span><span style="color: #007700">(</span><span style="color: #0000BB">$node</span><span style="color: #007700">, </span><span style="color: #0000BB">FALSE</span><span style="color: #007700">, </span><span style="color: #0000BB">TRUE</span><span style="color: #007700">);<br /></span><span style="color: #0000BB">?&gt;</span></span>

Now, quick! Who can tell me what passing FALSE as the second argument and TRUE as the third argument means?

read more

Comments

Posted on by cora guo (not verified).

the first FALSE means "do not show node teaser as node content when viewing a node, display node body only"
the last TRUE means "the node will be displayed by itself as a page"

tell me if i made some mistakes
:)

cora

Posted on by Anonymous (not verified).

Using an IDE I can see the parameters after writing the function name (CTRL+space or something), also I can see the descriptions of the parameters...

I don't think this should be banned at all....

Posted on by Anonymous (not verified).

There are others DX issues more important than this one, this is very, very minor if you have the functions parameters documented.

Posted on by Tj Holowaychuk (not verified).

I am not sure I would consider this a huge problem either really seeing as for the most part we do a decent job documenting, but I would agree that perhaps constants passed as a single argument or bitmask flags may be a little more readable

Posted on by Nick Lewis (not verified).

What I find even weirder is that it makes this possible:

<?php
return node_view($node, TRUE, TRUE);
?>

Why would you want a node_view to be both a teaser and a page? If you answered, "Because you have no idea what you are doing..." you'd be correct.

That said, the way the system treats pages and teasers is completely different -- so different that I don't know weather this is appropriate:

<?php
// though it would be more flexible to let you do whatever for the $op, so you aren't limited to two types of views...
function node_view($node, $op = 'teaser') {
//...
}
print
node_view($node, "page");
?>

I could live with this however:

<?php
function node_view($node, $page = false) {
}
//Even better would be simply this:
// "view" is a full page view
$node->view();
// teaser allows you to bypass the normal theme functions for special cases
$node->teaser('some_theme_function');
?>