Skip to main content

PHP Code Review Guidelines

General
  •  The code works
  •  The code is easy to understand
  •  Follows coding conventions
  •  Names are simple and if possible short
  •  Names are spelt correctly
  •  Names contain units where applicable
  •  There are no usages of magic numbers
  •  No hard coded constants that could possibly change in the future
  •  All variables are in the smallest scope possible
  •  There is no commented out code
  •  There is no dead code (inaccessible at Runtime)
  •  No code that can be replaced with library functions
  •  Variables are not accidentally used with null values
  •  Variables are immutable where possible
  •  Code is not repeated or duplicated
  •  There is an else block for every if clause even if it is empty
  •  No complex/long boolean expressions
  •  No negatively named boolean variables
  •  No empty blocks of code
  •  Ideal data structures are used
  •  Constructors do not accept null/none values
  •  Catch clauses are fine grained and catch specific exceptions
  •  Exceptions are not eaten if caught, unless explicitly documented otherwise
  •  Files/Sockets and other resources are properly closed even when an exception occurs in using them
  •  null is not returned from any method
  •  == operator and === (and its inverse !==) are not mixed up
  •  Floating point numbers are not compared for equality
  •  Loops have a set length and correct termination conditions
  •  Blocks of code inside loops are as small as possible
  •  No methods with boolean parameters
  •  No object exists longer than necessary
  •  No memory leaks
  •  Code is unit testable
  •  Test cases are written wherever possible
  •  Methods return early without compromising code readability
  •  Performance is considered
  •  Loop iteration and off by one are taken care of

Architecture

  •  Design patterns if used are correctly applied
  •  Law of Demeter is not violated
  •  A class should have only a single responsibility (i.e. only one potential change in the software's specification should be able to affect the specification of the class)
  •  Classes, modules, functions, etc. should be open for extension, but closed for modification
  •  Objects in a program should be replaceable with instances of their subtypes without altering the correctness of that program
  •  Many client-specific interfaces are better than one general-purpose interface.
  •  Depend upon Abstractions. Do not depend upon concretions.

API

  •  APIs and other public contracts check input values and fail fast
  •  API checks for correct oauth scope / user permissions
  •  Any API change should be reflected in the API documentation
  •  APIs return correct status codes in responses

Logging

  •  Logging should be easily discoverable
  •  Required logs are present
  •  Frivolous logs are absent
  •  Debugging code is absent
  •  No print_r, var_dump or similar calls exist
  •  No stack traces are printed

Documentation

  •  Comments should indicate WHY rather that WHAT the code is doing
  •  All methods are commented in clear language.
  •  Comments exist and describe rationale or reasons for decisions in code
  •  All public methods/interfaces/contracts are commented describing usage
  •  All edge cases are described in comments
  •  All unusual behavior or edge case handling is commented
  •  Data structures and units of measurement are explained

Security

  •  All data inputs are checked (for the correct type, length/size, format, and range)
  •  Invalid parameter values handled such that exceptions are not thrown
  •  No sensitive information is logged or visible in a stacktrace

Comments

Popular posts from this blog

MySQL 4.1+ using old authentication

When I was working with XAMPP in Ubuntu and asked write PHP script to connect to remote MySQL server which is using PASSWORD hash function to save the password for user, and I found following error. Warning: mysql_connect() [function.mysql-connect]: Premature end of data (mysqlnd_wireprotocol.c:554) in path/to/the/file/where/connection/script/is/written/ Warning: mysql_connect() [function.mysql-connect]: OK packet 1 bytes shorter than expected in path/to/the/file/where/connection/script/is/written/ Warning: mysql_connect() [function.mysql-connect]: mysqlnd cannot connect to MySQL 4.1+ using the old insecure authentication. Please use an administration tool to reset your password with the command SET PASSWORD = PASSWORD('your_existing_password'). This will store a new, and more secure, hash value in mysql.user. If this user is used in other scripts executed by PHP 5.2 or earlier you might need to remove the old-passwords flag from your my.cnf file in path/to/the/file/where/conn...

The 7 Types of Leadership: Inspiring Examples and Insights

Leadership is a multifaceted concept with various styles that cater to different needs and situations. Understanding these styles can help you develop your leadership skills and adapt to diverse scenarios. In this article, we'll explore seven prominent leadership styles, each accompanied by insightful examples from influential authors. Let's dive in! Type 1: Transformational Leadership 🚀 Transformational Leadership: Inspiring Change and Innovation Transformational leadership focuses on inspiring and motivating followers to achieve extraordinary outcomes and, in the process, develop their own leadership capacity. Example from Author: James MacGregor Burns James MacGregor Burns, in his book "Leadership," describes transformational leaders as those who seek to change the status quo by appealing to their followers' values and sense of higher purpose. 🔸 Characteristics: - Inspirational Motivation - Intellectual Stimulation - Individualized Consideration - Idealized I...

Security: Password Hashing

In this article I'm going to cover password hashing, a subject which is often poorly understood by newer developers. Recently I've been asked to look at several web applications which all had the same security issue - user profiles stored in a database with plain text passwords. Password hashing is a way of encrypting a password before it's stored so that if your database gets into the wrong hands, the damage is limited. Hashing is nothing new - it's been in use in Unix system password files since long before my time, and quite probably in other systems long before that. In this article I'll explain what a hash is, why you want to use them instead of storing real passwords in your applications, and give you some examples of how to implement password hashing in PHP and MySQL. Foreword As you read on you'll see that I advocate the use of a hashing algorithm called Secure Hashing Algorithm 1 (or SHA-1). Since I wrote this article, a team of researcher...