In this video I once again come back to the idea of good code. I present 4 characteristics of good code together with examples and I talk about 3 questions that I always ask when I finish writing or reviewing code.
Notes
Introduction
A few episodes ago I talked about the concept of good code. I explained 4 characteristics of good code. Then again in episode number 4 I showed a few simple techniques that can improve the readability of your code. Today again I am going to talk about good code and I will focus on a few characteristics of well designed code. For each characteristic I will show a few examples and I will explain how to improve that code.
Honest
- someone honest is someone who tells the truth (does not lie) and tells the whole truth (does not hide parts of it)
- honest code does not lie (e.g. a method called "createUser" that actually deletes user is a lie)
- honest code does not hide the truth (e.g. a method called "findUser" that creates a new user when it can't be found, hides the truth, variable called "data" does not reveal what kind of data it stores)
class User
def self.find(id)
user = @users.detect { |user| user.id == id }
return user || self.new()
end
end
class User
def self.find(id)
return @users.detect { |user| user.id == id }
end
end
User.find(id) || User.new()
class User
def self.find_or_new(id)
user = @users.detect { |user| user.id == id }
return user || self.new()
end
end
class Http {
public static function get(url, filter = null) {
return fetch(url)
.then((resp) => resp.json())
.then((json) => filter ? filter(json) : json);
}
}
class Http {
public static function get(url, filter = null) {
return fetch(url)
.then((resp) => resp.json())
}
}
Http.get("<http://someurl.com>").then((json) => filterData(json));
class Http {
public static function getAndFilter(url, filter = null) {
return fetch(url)
.then((resp) => resp.json())
.then((json) => filter ? filter(json) : json);
}
}
class User {
public allUsers;
public static function create(email, password) {
return User.allUsers[0];
}
}
Concise
- good code should not be unnecessarily long
- as always there's a balance here, so the point is to aim for making code easy to understand
- code that might be too short:
- 200-char one-liner probably won't be easy to understand
- variables that use acronyms
- code that might be too long
- writing every operation in separate line with a new variable
<?php
function totalPrice($order)
{
$items = $order->items;
$items = new Collection($items);
$prices = $items->map(function($item) { return $item->price * $item->count });
$totalPrice = $prices->reduce(function($total, $price) {
return $total + $price;
}, 0);
return $totalPrice;
}
<?php
function totalPrice($order)
{
return (new Collection($order->items))
->map(function($item) { return $item->price * $item->count })
->reduce(function($total, $price) { return $total + $price; }, 0);
}
<?php
function totalPrice($order)
{
return (new Collection($order->items))
->reduce(function($total, $item) {
return $total + ($item->price * $item->count)
}, 0);
}
Unobtrusive
- code should not impact the places that are not directly related to it
- changes should be limited to the layer on which you are operating
- examples:
- generating HTTP routes in the model layer
- running database query in the template
class UserController {
public handleRequest(request: Request): View {
return User.handle(request);
}
}
class User {
public static handle(request: Request): View {
if (request.method === "POST") {
user = User.create(request.parameters);
return new View("createUser", user);
} else {
user = User.find(request.query["id"]);
return new View("showUser", user);
}
}
public static create(parameters: any) {
// TODO
}
public static find(id: number) {
// TODO
}
}
class UserController {
public handleRequest(request: Request): View {
if (request.method === "POST") {
return this.handlePostRequest(request);
} else {
return this.handleGetRequest(reqeust);
}
}
private handleGetRequest(request: Request): View {
user = User.find(request.query["id"]);
return new View("showUser", user);
}
private handlePostRequest(request: Request): View {
user = User.create(this.filteredParameters(request.parameters));
return new View("createUser", user);
}
private filterRequestParameters(parameters: any) {
// TODO
}
}
class User {
public static create(parameters: any) {
// TODO
}
public static find(id: number) {
// TODO
}
}
Consistent
- the way your application works should be the way users expect it to work
- the same applies to code, it should look and work the way developers expect it to
- inconsistent code breaks that assumption
- consistency comes in many ways: naming, language idioms, using the same libraries, etc.
- example:
class User
def self.find(id = nil)
return @users.find { |user| user.id == id }
end
end
class Product
def self.find(id: nil)
product = @products.find { |p| p.id == id }
raise NotFoundException unless product
return product
end
end
user = User.find(10) # will return User object or nil
product = Product.find(id: 20) # will return Product object or raise an error
class User
def self.find(id = nil)
return @users.find { |user| user.id == id }
end
end
class Product
def self.find(id = nil)
return @products.find { |p| p.id == id }
end
end
user = User.find(10) # will return User object or nil
product = Product.find(20) # will return Product object or nil
Ask yourself: is this code easy to modify?
- as developers we often know that our code will change at some point
- we don't know when, we don't know in what way, we just know it will change
- a code is easy to modify when it is:
- easy to understand (we can't change what we don't know)
- easy to navigate (we can't change what we can't find)
- easy to test (we can't change what we can't verify)
Top comments (0)