1. Magic string in code should be replaced by class constants
Advantages: development environment will offer the constant name, the value is changed in one place. Wrong:public static function getPlayMoneyLimit() {
$pointsLimit = self::poker_get_option('RequestPlayMoneyLimit');
return (int) $pointsLimit->message();
}
public static function getPlayMoneyLimit() {
$pointsLimit = self::poker_get_option(self::REQUEST_PLAY_MONEY_LIMIT_OPTION);
return (int) $pointsLimit->message();
}
2. Settings should be placed in a separate config file and not stored in the code.
For example, logins, passwords, file paths, application parameters. All that needs to be changed when configuring the application in another location. Wrong (some_file.php):define('PAYMENT_QUICKPAY_MERCHANT_ID', '5afab57b-33ef-4c59-03cd-9f7844f53118');
define('PAYMENT_QUICKPAY_WEBSITE_ID', '346baa5d-265a-479c-c588-4c6ee7c5762d');
payment_quickpay_merchant_id = 5afab57b-33ef-4c59-03cd-9f7844f53118
payment_quickpay_website_id = 346baa5d-265a-479c-c588-4c6ee7c5762d
3. The function should return the data of only one type or null
Wrong:function dateField($dateField = null){
if (!isset($this->_dateField)) {
$this->_dateField = '';
}
if (is_null($dateField)) {
return false;
}
else {
$this->_dateField = $dateField;
}
return $this->_dateField;
}
function dateField($dateField = null){
if (!isset($this->_dateField)) {
$this->_dateField = '';
}
if (is_null($dateField)) {
return null;
}
else {
$this->_dateField = $dateField;
}
return $this->_dateField;
}
4. Erratums should be corrected immediately, otherwise code completion will spread them throughout the project
function headDefatults(){
$re = array();
$heads = array_keys($this->headArray());
foreach ($heads as $key){
$re[$key] = 0;
}
return $re;
}
function dataDefaults($label){
$re = array(
'label' => $label,
'data' => $this->headDefatults(),
'total' => 0,
);
return $re;
}
5. Timestamp as the array key should be used only in case of 100% certainty that there won’t be two or more events occurred in one second
6. You should check the existence of the key before using it in public methods
Otherwise “Notice: Undefined index:”. In private and protected methods checking can be omitted if we are 100% sure that this key is there. Wrong:public static function createTable($data)
{
$password = $data['password'];
unset($data['password']);
$tableInfo = new TableInfo();
…
}
public static function createTable($data)
{
if (isset($data['password'])) {
$password = $data['password']; }
else {
$password = '';
}
unset($data['password']);
$tableInfo = new TableInfo();
…
}
7. Javascript. Verifying the existence of a variable (defined) should be done using typeof
Wrong:setBlockId : function(blockId) {
if (blockId != undefined) {
this._blockId = blockId;
}
else{
this._blockId = this.DEFAULT_BLOCK_ID;
}
},
setBlockId : function(blockId) {
if (typeof blockId != 'undefined') {
this._blockId = blockId;
}
else{
this._blockId = this.DEFAULT_BLOCK_ID;
}
},
8. Exceptions should be logged but not be displayed for the user
Wrong:public function getActionsListAsJson()
{
$serializer = Zend_Serializer::factory('Json');
try {
$actionsIds = array();
foreach($this->getActionList() as $action){
$actionsIds[] = $action->getSoldObject();
}
return $serializer->serialize($actionsIds);
} catch (Zend_Serializer_Exception $e) {
echo $e;
}
}
public function getActionsListAsJson()
{
$serializer = Zend_Serializer::factory('Json');
try {
$actionsIds = array();
foreach($this->getActionList() as $action){
$actionsIds[] = $action->getSoldObject();
}
return $serializer->serialize($actionsIds);
} catch (Zend_Serializer_Exception $e) {
Logger::log($e);
}
}
9. Escape variables in the html-templates
Wrong:<html>
<head>
</head>
<body>
Player <?php echo $player->f_family_name . ' ' . $player->f_name . ' (' . $player->f_nick . ')'; ?> has uploaded new avatar.
</body>
</html>
<html>
<head>
</head>
<body>
Player <?php echo $this->escape($player->f_family_name) . ' ' . $this->escape($player->f_name) . ' (' . $this->escape($player->f_nick) . ')'; ?> has uploaded new avatar.
</body>
</html>
10. Optimistic assumption about the success of execution. You should check the result and respond to the error
Wrong:public function clearApprovedAvatar() {
$this->_davRequest($this->getApprovedAvatarUrl(), 'DELETE');
return true;
}
public function clearApprovedAvatar() {
if ($this->_davRequest($this->getApprovedAvatarUrl(), 'DELETE')) {
return true;
};
Logger::log('Error on deletion ' . $this->getApprovedAvatarUrl());
return false;
}
