From RNWiki
Jump to: navigation, search

Process>Coding Standards

These pages are not meant to be taken by the general public as "end-all" static statements of direction. They are more "guiding principles" and they can and will change over time. We make these available here so the RavenNuke Team has one place to go to for this type of information, when decisions on direction have been made, and we decided to give the "public" a "peek" into what we are trying to accomplish... a "process roadmap" if you will.

NOTE: These standards do NOT apply to code that is incorporated into RavenNuke unless the designated maintainer desires to re-code (including updates) OR the code being incorporated is no longer being supported by the author. In the latter case, the code becomes "core" to RavenNuke, therefore, it should be upgraded to Standard wherever possible.

You will note that we intend for this to be light-hearted! How else are we to have fun with "standards". :-)

Stack Requirements

With the 2.5.0 release, the following minimum stack is required for development, QA testing, and is what is going to be supported:

  • Apache 1.3.3x, 2.x
  • MySQL 5.0
  • PHP 5.2
  • On either Windows or Linux platform

XAMPP is an excellent packaged version of all of the above and is available for Windows and Linux (as well as other platforms).

While older versions of the Stack requirements may work, they are not guaranteed to work and are not actively supported.

Indenting and Line Length

Tabs shall be used instead of spaces for the following reasons:

  1. Although traditionally an issue with CVS (according to PEAR), does not seem to affect Subversion.
  2. Too easy to forget to set desired standard tab stops in editors which replace spaces for tabs, which end up causing our indentation to be inconsistent (sometimes 2 spaces, other times 3 or 4 or __).
  3. In some editors used by the Team (such as NuSphere PhpED), spaces are more difficult to "ignore" for indentation purposes.
  4. Because this is what Raven has asked us to do - LOL.

Just use good judgment in terms of line lengths. Remember that having to look through unwrapped (and even auto-wrapped code) can be VERY difficult to digest. Most of us now use better text editors or IDE's and have much larger screens that the "old days", so we can go a little longer, but, again, use good judgment. (And if you are reading this and you are really h___ bent on using command line vi editor... tough!)

Line Termination

Line termination follows the Unix text file convention. Lines must end with a single linefeed (LF) character and please do not use carriage returns (CR) as is the convention in Apple OS's or the carriage return - linefeed combination (CRLF) as is standard for the Windows OS.

String Parsing vs. Concatenation

This one is a performance play. String concatenation is faster than string parsing. (Some PHP experts are saying that this has been greatly eliminated in PHP5.2, but why even mess with "greatly"... we've noticed the difference and so have our customers.) So, wherever possible, use something like this:

$sql = 'SELECT * FROM `' . $prefix . '_users` WHERE `user_id` = \'' . $id . '\'';

instead of this:

$sql = "SELECT * FROM `$prefix_users` WHERE `user_id` = '$id'";

See that the first example is nicely formatted with spaces in-between the "dot" concatenation operator and with spaces around the "=" signs. It just helps with readability when trying to debug a long concatenated string.

Here is another nice tid-bit from Gremmie regarding string concatenation while using echo. Do this:

echo 'I am a long string var with ', $somevar, ' inserted in the middle.', $canhaveasmanyasyouwant, ...;

Instead of:

echo 'I am a long string var with ' . $somevar . ' inserted in the middle.' . $canhaveasmanyasyouwant . ...;

This is faster as echo actually takes any number of arguments. Much better to take advantage of a PHP construct than to use a function (i.e., concatenation).

As with everything, though, there are trade-offs. The real key here is that if something just makes better sense to use, such as the very slow HEREDOC syntax for large blocks of HTML with lots of variable replacements, use the tool that fits the job! HEREDOC syntax should be used for including inline HTML instead of breaking out of PHP every time. Echo's are sufficient for the smaller HTML code/fragments, but if you have a block of HTML code HEREDOC is much less cumbersome and more easily maintained thank an ugly string of code, quotes, tick marks, etc.

Related Assignment "Blocking"

In most cases, there should be only one space before and after the equal sign ("=") in an assignment. Some would argue that with blocks of related assignments, that the following might be desired:

$short         = foo($bar);
$long_variable = foo($baz);

This can cause issues when one wants to perform a global search and replace of a variable name (unless the substituted variable is of exactly the same length). There really is no need to have the extra spaces and it is slightly less script file size and parsing. However, please, NEVER use tabs for formatting code outside of just left-hand indentation!

Control Structures

Sticky subject for most. The key is not to get so entangled in "it has to be this way" that we don't want to write "tight" or "elegant" code. Again, a judgment call as to whether to use more "short-hand" code; others can stand to learn how to write more elegant code - wink - if there is a legitimate purpose, such as performance.

Here are some sample control structures, taken in their most complicated form, to give you an idea of what we're after (control statements should have one space between the control keyword and opening parenthesis, to distinguish them from function calls).

IF-THEN-ELSE/ELSEIF

if ((condition1) || (condition2)) {
    action1;
} elseif ((condition3) && (condition4)) {
    action2;
} else {
    defaultaction;
}


The "default" condition should be placed at the top of the "SWITCH" structure.

SWITCH

switch (condition) {
default:
    defaultaction;
    break;

case 1:
    action1;
    break;

case 2:
    action2;
    break;
}

All other control structures should follow suit.

Function Calls

Functions should be called with no spaces between the function name, the opening parenthesis, and the first parameter; spaces between commas and each parameter, and no space between the last parameter, the closing parenthesis, and the semicolon. Here's an example:

$var = foo($bar, $baz, $quux);

Function Definitions

It is pretty much an accepted "standard" that arguments with default values should go at the end of the function argument list. This is possibly a carry over from JavaScript requiring it, at least in some earlier versions. It is also somewhat more aesthetic to the eye not having to process back-and-forth switching from non-defaulted values to defaulted ones. So, we should accept this as a standard. It is a good idea to always give consideration to providing default values as an added security measure - i.e., you can count on the value if one is not provided. It would also be good to give consideration to passing variables by reference. There is a trade-off here, though, yes, we get increased performance and decreased RAM profile, but now one has to be EXTRA careful modifying values!

function connect(&$dsn, $persistent = false) {
    //... code goes here
}

NOTE: Objects in PHP5 are always passed by reference (this was not so in PHP4).

Another consideration with functions is to return a meaningful value if one is appropriate. Again, trying to ensure good control over both inputs and outputs of the function and ensure consistency for their "constituents".

Class Definitions

Not really all that different from Function Definitions...

class Foo_Bar {
    //... code goes here
}

Comments

Code documenting standards will be developed elsewhere, but just a quick note about commenting style. Because many text/code editors do not properly highlight PHP syntax when the Perl/Shell script type comment of "#" is used, and will actually cause the editor a migraine and throw off all code coloring from that point on, we will only use C/C++ style comments. So, like these:

/* Nice comments */
// Nice comments
/**
 * Even nicer comments if we're going to use PHPDocumentor
 */
# Not so nice... everything below here gets all syntax hosed up.  :-(

Including Code

It is actually claimed (and proven) that on very busy sites that only include/require should be used (see http://pear.php.net/manual/en/pear2cs.php). However, we prefer the extra security aspect of using include_once and require_once. It may also be a lesser known fact that these are not functions! Therefore, the following are better examples of usage:

include_once 'header.php';
require_once 'mainfile.php';
require_once INCLUDE_PATH . 'custom_files/custom_head.php';

PHP Code Tags

Use <?php ?> instead of <? ?> for portability purposes.

Naming Conventions

Naming conventions are important in a large CMS system such as RavenNuke(tm) in order to help reduce variable "collisions" within the CMS and/or with "plug-ins". With PHP5 and use of classes and namespaces, this can be greatly reduced or eliminated, but we're far from that situation at the moment.

Filenames All files should only contain only alphanumeric characters, underscores and dash character ("-") are permitted but spaces are strictly prohibited in all cases. Any file that contains PHP code MUST always end with the extension ".php".

Functions Function names may only contain alphanumeric characters. Underscores are not permitted. Numbers are allowed in function names but are not recommended. They must always start with a lowercase letter and when a function name consists of more than one word, the first letter of each new word must be capitalized. This practice is commonly called "CamelCase" formatting Get wise.

Function names should be as verbose as is practical to fully describe their purpose and behavior.

Variables Just as function names, variable names may only contain alphanumeric characters. Underscores are not permitted. Numbers are permitted but not recommended. Variable names must always start with a lowercase letter and follow the "CamelCase" Get wise.

Always remember to name variables in such a way as to easily recognize/describe the type of data intended to store in them. Short and not descriptive variable names as "$i" are highly discouraged for all but the smallest loop contexts. For loops with more than 20 lines of code, the index variables should have more descriptive names.

Constants Constants may contain both alphanumeric characters and underscores. Numbers are permitted in constant names. All letters used in a constant name MUST be capitalized while all words MUST be separated by underscore characters. Speaking clearly... Constant names shall always be in UPPER_CASE_WITH_INDIVIDUAL_NODES_SEPARATED_LIKE_THIS. The key is the all CAPS.

NOTE: *nuke put underscores at the front of their language constants, but this is not really necessary.

This section needs work - as decisions are made in this regards, they should be documented here.

SQL Calls

Security related topics shall be handled in a separate section (see the MySQL Manual for security guidelines too), but the following shall be considered while preparing SQL to be executed and other standards around SQL calls:

Since our standard Database Management System (DBMS) is MySQL, the following are applicable:

  • All table names and field names should be encapsulated within back-tics, such as `nuke_authors` and `user_id`.
  • All constants should be encapsulated within single quotes, even numeric fields, such as `user_id` = '5'.

The core $db database object shall be used for all database calls. This keeps the number of DB connections down as well as standardizes on one abstraction layer to make swapping out later easier. NOTE: The older PHP-Nuke sql_layer ($dbi) was deprecated with version 2.3.0 and is no longer to be used. It will actually cause blank white pages to occur, so if this happens to you, there is a good chance this is the reason.

In order to assist with readability, especially within the dblog output, MySQL reserved words should be ALL CAPS with all other elements, such as table and field names, in all lower case. Here is an example of a well formatted SQL call:

SELECT * FROM `nuke_users` WHERE `user_id` = '5'

Although several $db statements can be strung together in order to tighten up the code, for readability and usability purposes, the following should be considered (an example of all these concepts is given below):

  • Don't try to do too much all in one PHP line (hampers readability).
  • Make sure and test for errors upon a db call.
  • Test for and handle every possible result - e.g., handle 0 rows, 1 or more rows, etc.
$sql = 'SELECT `did`, `doc_name`, `doc_text` FROM `' . $prefix . '_legal_docs` WHERE `doc_status` = \'1\' AND `language` = \'' . addslashes($lang) . '\'';
    if ($result = $db->sql_query($sql)) {
        if ($db->sql_numrows($result) > 0) {
            << handle the most likely normal case where proper results were returned >>
            << one could also handle single row return vs. multiple row return with a slight change to this >>
        } else {
            << handle the case where no rows were returned IF necessary to do so >>
    } else {
        << handle the case where the query was bad - i.e., had sql errors >>
    }

There are different ways to cycle through a result set and most are just fine. Here are a few pertinent examples:

while ($row = $db->sql_fetchrow($result)) {
    << use $row['field_name'] to reference the value >>
}

<< OR >>

while (list($var1, $var2, ..., $varN) = $db->sql_fetchrow($result)) {
    << use $var1, $var2, ... $varN to reference the returned column values >>
}

File Formats

All files shall be saved in ISO-8859-1 format with UNIX style line endings. (We are considering UTF-8 within the roadmap, but is no set release where this will implemented in.)

VERY IMPORTANT NOTE: We have had numerous issues reported over the years (mostly end-user caused by using bad text editors) with extra lines following the closing ?> line. Two different solutions have been proposed for this: 1) don't add the extra line - lol, and 2) don't include the closing ?> tag. We know that 1) works every time, but 2) might be a consideration. 2) is actually what MediaWiki does.

@ (Silent Die Error Handling)

Since error reporting has been made a switchable feature with the default out-of-the-box settings set to hide errors, we are now to move away from using the "@" to hide errors. Use of "@" makes it nearly impossible to debug site problems (white screen anyone?). Over time, we shall remove the use of the "@" and no more use of it should be added except when there is no other alternative. When the use of this cannot be avoided, alternate methods of error messaging and/or logging should be used. Eventually, we will have a standard API for error handling. Here are some sample ways of providing alternate error messaging:

if (file_exists('some_file.php')) require_once 'some_file.php';
else {
    echo 'some_file.php not found';
    die();  // Or redirect or whatever
}

$sql = 'SELECT * FROM `some_table`';
@mysql_query($sql) or die('Unable to execute query.  MySQL reports: ' . mysql_error());

Error Reporting/Logging Levels

Tremendous strides have been made to clear up any and all PHP Warnings within the code. In order to ensure these errors are found and cleaned up, and that new ones are not injected, development and testing shall always be done with $display_errors = true within config.php and with $error_reporting = E_ALL within rnconfig.php as a minimum. We should also turn on dblogging to at least the error reporting level (i.e., $loglevel = 1;) within rnconfig.php.

Ideally, we'd be working with an error handling class within PHP5 with more complete and robust capabilities (try/catch anyone?). Some day.

Stop the Insanity

<MONTEGO_RANT>

I have saved the best for last. Have you ever seen this within PHP-Nuke?

$somevar = "$anothervar";

We need to really stop this insanity as it holds NO value, security or otherwise, and is exactly equivalent to simple this:

$somevar = $anothervar;

Aaaaarrrggghhh... ok. Feel better now...

</MONTEGO_RANT>

<RAVEN_RANT>

Or, this one:

$somevar = "".SOME_CONSTANT."";

Should simply be:

$somevar = SOME_CONSTANT;

</RAVEN_RANT>

Deprecated HTML Tags

We should not be using the following tags in our presentation code.

Deprecated Replacement
< center > style="text-align: center;"
< s > style="text-decoration: line-through;"
< strike > style="text-decoration: line-through;"
< u > style="text-decoration: underline;"
< b > style="font-weight: bold;"
< i > style="font-style: italic;"
< font > < div > or < span >

Ultimately we should have classes within the ravennuke.css file for these.

File Security

If you want to stop users from accessing your file directory you should be using something similar to the following:

if (!defined('ADMIN_FILE')) {
	Header('Location: index.php');
	exit('Access Denied');
}
if (!defined('MODULE_FILE')) {
	Header('Location: index.php');
	exit('Access Denied');
}
if (!defined('NUKE_FILE')) {
	Header('Location: index.php');
	exit('Access Denied');
}

If the above three choices are not applicable you should be using this.

if (realpath(__FILE__) == realpath($_SERVER['SCRIPT_FILENAME'])) {
	header('Location: index.php');
	exit('Access Denied');
}

The above is a replacement for the following.

if (stristr(htmlentities($_SERVER['PHP_SELF']) , 'somefilename.php')) {
	Header('Location: index.php');
	exit('Access Denied');
}