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)