Good code, bad code
Tags: video, code, advice
Published: 2020-09-02 13:43:02

---
title: Good code, bad code
published: true
date: 2020-09-02 13:32:42 UTC
tags: video,code,advice
cover_image: https://www.notonlycode.org/content/images/2020/09/thumbnal.jpg
canonical_url: https://www.notonlycode.org/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.

{% youtube i60Hnhuf6EE %}

# 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)

```ruby
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
```

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

```typescript
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
<?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

```typescript
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:

```ruby
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)