DEV Community

Cover image for Good code, bad code
Gregory Witek
Gregory Witek

Posted on • Originally published at notonlycode.org on

Good code, bad code

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
Enter fullscreen mode Exit fullscreen mode
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);
  }
}
Enter fullscreen mode Exit fullscreen mode
class User {
  public allUsers;

  public static function create(email, password) {
    return User.allUsers[0];
  }
}
Enter fullscreen mode Exit fullscreen mode

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);
}

Enter fullscreen mode Exit fullscreen mode

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
  }

}

Enter fullscreen mode Exit fullscreen mode

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

Enter fullscreen mode Exit fullscreen mode

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)