Programming Best Practices

Sure, you can/should avoid needlessly inefficient code. However, don't make the mistake of preemptively optimizing everything. Many of your functions won't be called in performance critical situations. It's far more important for your code to be clear and understandable than it is for every last function to be optimized with obfuscated logic and code.

A) When you start writing the code, you're likely to be wrong about where the bottlenecks are, and you'll spend time optimizing something completely irrelevant when some other function is what really needs your attention.

B) The complicated logic for your "optimization" might be optimizing for the wrong problem. Maybe the really slow thing is you have a query that needs a DB index, and the processing on the results of that query are dwarfed by the cost of retrieving the data in the first place. You really need to understand what the problem is before you try to fix it.

C) The additional complication in your code will make debugging or extending the code more difficult. In the vast majority of cases, the extra cost in human developer time far outweighs the minor savings in computational power that your "optimization" made. Computers are fast and cheap. Humans are slow and expensive.

D) Correctness is always more important than speed. First write your code so it provides the correct results. Often, that's enough of a challenge. ;) Once it actually does what it's supposed to, you can use a profiler to see if there are problem spots that can be fine tuned. But focus on correctness first.

User input and what your code prints out are basically the root of all security problems. Pay EXTRA attention any time your code is touching user-supplied data or is generating output.

If your code is touching the filesystem at all you should be extra careful about that, too.

Oh, and whenever your code is generating output, consider using a theme function. ;)

in hook_update_N() functions, we usually have queries to performs. Sometimes however, there is no query but some other operation is performed (like updating some variables). Instead of returning an empty array, return some useful message this way:

A parsing error is immediately visible and easily fixed. A wrong variable attribution can remain hidden for a long time and be hard to debug.

How often have you written if ($foo = 'bar') {} when you meant: if ($foo == 'bar') {}?? Me? Very often.

Particularly with OO. Create static caching factory functions rather than global variables to avoid name space collisions.

Would also recommend teaching people when to use static variable caching instead of Session variables.

For instance the the l() function already uses check_plain so do not use check_plain for the text parameter you pass it.

One should store any keyed or aggregated text in the database using check_plain so that check_plain isn't needed to display the data from the DB.

This comment is mainly in relation to the comment above about the practice of writing "if ('bar' == $foo) {}" for easier debugging but it applies generally to what this document should be about. A Best Practices guide should focus on explaining what constitutes superior code - I take superior to mean things like "more efficient", "more logical". In the case the "if ('bar' == $foo) {}" practice, any efficiency gained is efficiency in the face of error-prone-ness, rather than efficiency of the code itself, and so combined with the fact that it is logically awkward, I would say it does not belong in a Best Practices guide but more in a different, probably equally helpful but less authoritative, sort of document. It would be great if we could keep this one for more universally agreed upon practices.

Any thoughts on the ternary operator? Does it make code harder to read? Is it faster than a regular if-else statement?