Do not take more than two function arguments
Limiting the number of arguments to a function is very important because it makes your function easier to test. More than three parameters results in too many combinations of parameters, and you have to test a lot of different cases for each individual parameter.
No parameters is optimal, one or two parameters are acceptable, and more than three should be avoided. That’s important. If you have more than two arguments, your function is probably trying to do too much; if not, you may need to pass in a higher-level object as an argument.
Bad:
function createMenu($title, $body, $buttonText, $cancellable)
{
// ...
}Copy the codeGood:
class MenuConfig
{
public $title;
public $body;
public $buttonText;
public $cancellable = false;
}
$config = new MenuConfig();
$config->title = 'Foo';
$config->body = 'Bar';
$config->buttonText = 'Baz';
$config->cancellable = true;
function createMenu(MenuConfig $config)
{
// ...
}Copy the codeA function only does one thing
This is an important principle in software engineering. This will make your code clear and easy to understand and reuse.
Bad:
function emailClients($clients)
{
foreach ($clients as $client) {
$clientRecord = $db->find($client);
if($clientRecord->isActive()) { email($client); }}}Copy the codeGood:
function emailClients($clients)
{
$activeClients = activeClients($clients);
array_walk($activeClients, 'email');
}
function activeClients($clients)
{
return array_filter($clients, 'isClientActive');
}
function isClientActive($client)
{
$clientRecord = $db->find($client);
return $clientRecord->isActive();
}Copy the codeWhat does the function name say about what it does
Bad:
class Email
{
/ /...
public function handle(a)
{
mail($this->to, $this->subject, $this->body);
}
}
$message = newEmail(...) ;// What is this? Handle to a message? Or do YOU want to write a file? (Reader's question)
$message->handle();Copy the codeGood:
class Email
{
/ /...
public function send(a)
{
mail($this->to, $this->subject, $this->body);
}
}
$message = newEmail(...) ;//
$message->send();Copy the codeFunctions should only have one layer of abstraction
When you have multiple levels of abstraction, your functions are already doing too much. Breaking up these functions makes your code more reusable and more testable. Bad:
function parseBetterJSAlternative($code)
{
$regexes = [
// ...
];
$statements = explode(' ', $code);
$tokens = [];
foreach ($regexes as $regex) {
foreach ($statements as $statement) {
// ...
}
}
$ast = [];
foreach ($tokens as $token) {
// lex...
}
foreach ($ast as $node) {
// parse...}}Copy the codeBad Too: We moved some work out of the function, but the parseBetterJSAlternative() function is still too complex to test.
function tokenize($code)
{
$regexes = [
// ...
];
$statements = explode(' ', $code);
$tokens = [];
foreach ($regexes as $regex) {
foreach ($statements as $statement) {
$tokens[] = / *... * /; }}return $tokens;
}
function lexer($tokens)
{
$ast = [];
foreach ($tokens as $token) {
$ast[] = / *... * /;
}
return $ast;
}
function parseBetterJSAlternative($code)
{
$tokens = tokenize($code);
$ast = lexer($tokens);
foreach ($ast as $node) {
// parse...}}Copy the codeGood:
The best solution is to remove the dependency of the parseBetterJSAlternative function
class Tokenizer
{
public function tokenize($code)
{
$regexes = [
// ...
];
$statements = explode(' ', $code);
$tokens = [];
foreach ($regexes as $regex) {
foreach ($statements as $statement) {
$tokens[] = / *... * /; }}return$tokens; }}class Lexer
{
public function lexify($tokens)
{
$ast = [];
foreach ($tokens as $token) {
$ast[] = / *... * /;
}
return$ast; }}class BetterJSAlternative
{
private $tokenizer;
private $lexer;
public function __construct(Tokenizer $tokenizer, Lexer $lexer)
{
$this->tokenizer = $tokenizer;
$this->lexer = $lexer;
}
public function parse($code)
{
$tokens = $this->tokenizer->tokenize($code);
$ast = $this->lexer->lexify($tokens);
foreach ($ast as $node) {
// parse...}}}Copy the codeDo not use flags as arguments to functions
When you use flags as arguments in a function, your function doesn’t just do one thing, which goes against our previous rule that each function only does one thing, so don’t use flags as arguments.
Bad:
function createFile($name, $temp = false)
{
if ($temp) {
touch('./temp/'.$name);
} else{ touch($name); }}Copy the codeGood:
function createFile($name)
{
touch($name);
}
function createTempFile($name)
{
touch('./temp/'.$name);
}Copy the codeAvoid side effects
If a function does something other than “take a value and return a value or multiple values,” then the function can have side effects, such as accidentally writing to a file, modifying global variables, or sending money to a stranger.
Now suppose you do want to do something in a function that might have a side effect. For example, to write a file, all you need to do is to write the file in one place, rather than operating on the same file in several functions or classes, implementing a service (function or class) to operate on it, one and only.
The key is to avoid common pitfalls such as sharing state between unstructured objects, using mutable data types that can be written to any value, and not centralizing operations that can have side effects. If you can do that, you’ll be happier than most programmers.
Bad:
// Global variable referenced by following function.
// If we had another function that used this name, now it'd be an array and it could break it.
$name = 'Ryan McDermott';
function splitIntoFirstAndLastName(a)
{
global $name;
$name = explode(' ', $name);
}
splitIntoFirstAndLastName();
var_dump($name); // ['Ryan', 'McDermott'];Copy the codeGood:
function splitIntoFirstAndLastName($name)
{
return explode(' ', $name);
}
$name = 'Ryan McDermott';
$newName = splitIntoFirstAndLastName($name);
var_dump($name); // 'Ryan McDermott';
var_dump($newName); // ['Ryan', 'McDermott'];Copy the codeDo not modify global variables
Global contamination is a bad practice in many programming languages, because your library may conflict with another library, but your library’s users have no idea until an exception breaks in production. Let’s consider an example: What if you want to get a configuration array? You could write a global function, such as config(), but it might conflict with another library that tries to do the same thing.
Bad:
function config(a)
{
return [
'foo'= >'bar']},Copy the codeGood:
class Configuration
{
private $configuration = [];
public function __construct(array $configuration)
{
$this->configuration = $configuration;
}
public function get($key)
{
return isset($this->configuration[$key]) ? $this->configuration[$key] : null;
}
}
$configuration = new Configuration([
'foo'= >'bar',]);Copy the codeAvoid conditional judgment
People ask, “What should I do without the if statement?” The answer is that in many cases you can achieve the same effect with polymorphism. So what’s the advantage of doing that? Again, “a function should only do one thing.” When you have an if statement in your class or function, your function doesn’t just do one thing.
Bad:
class Airplane
{
// ...
public function getCruisingAltitude(a)
{
switch ($this->type) {
case '777':
return $this->getMaxAltitude() - $this->getPassengerCount();
case 'Air Force One':
return $this->getMaxAltitude();
case 'Cessna':
return $this->getMaxAltitude() - $this->getFuelExpenditure(); }}}Copy the codeGood:
interface Airplane
{
// ...
public function getCruisingAltitude(a);
}
class Boeing777 implements Airplane
{
// ...
public function getCruisingAltitude(a)
{
return $this->getMaxAltitude() - $this->getPassengerCount(); }}class AirForceOne implements Airplane
{
// ...
public function getCruisingAltitude(a)
{
return $this->getMaxAltitude(); }}class Cessna implements Airplane
{
// ...
public function getCruisingAltitude(a)
{
return $this->getMaxAltitude() - $this->getFuelExpenditure(); }}Copy the code