【Clean Code】Chapter3 Functionsのメモ

Clean Codeという本を読み始めた。

www.amazon.co.jp

友人から譲っていただいたもので（値段が高い。。）英語で書かれたものなので、読むのに時間がかかるかもしれないが、チームのコードレビューで活かせそうなので、読みながらメモしていく。

Small!

functionは短ければ短いほど良い。
かのKent BeckがSparkleで書いたコードは、全てのfunctionが3~5行くらいで書かれていた。（ので、それくらいで書くのが望ましい）

Do One Thing

functionは1つのことをしろ…というが、1つのこととは何だろう。
抽象レベルを1つだけ上げてfunctionの役割を説明した時に、一言で済むようにするのが良い。（実際の仕事としては1つではなくて良い）
細かく分けすぎてもだめ。同じような実装が存在する別のfunctionが存在する場合は、そのfunctionの役割を考え直す。
functionの中の処理をsectionで分けるケースも存在する。

One Level of Abstraction per Function

実装する時は、書いている処理がcore conceptなのか、detail implementionなのか区別すること。
一つのfunctionの中に書く処理は、core conceptとdetail implementionは一緒に書くべきじゃない。

Switch Statement

switch構文は常に冗長になりがちでDo One Thingの法則を守ることができない。
1. コード量が多く、Employee typeが増えるにつれて肥大化していく。
2. 一つ以上のことを指定いるし、単一責任原則を破っている
3. 開放/閉鎖原則を破っている
4. 似たようなfunctionがたくさん生成されてしまう。
-> Factoryパターンで対応しよう。（ポリモフィズムを使用しよう）

public Money calculatePay(Employee e) throws InvalidEmployeeType {
    switch (e.type) {
        case COMMISSIONED:
            return calculateCommisionedPay(e);
        case HOURLY:
            return calculateHourlyPay(e);
        case SALARIED:
            return calculateSalaliedPay(e);
        default:
            throw new InvalidEmployeeType(e.type);
    }
}

public abstract class Employee {
  public abstract boolean isPayday();
  public abstract boolean Money calculatePay();
  public abstract void deliverPay(Money pay);
}
-----------
public interface EmployeeFactory {
  public Employee makeEmployee(EmployeeRecord r) thorws InvalidEmployeeType;
}
-----------
public class EmployeeFactoryImpl implements EmployeeFactory {
  public Employee makeEmployee(EmployeeRecord r) throws InvalidEmployeeType {
    switch (r.type) {
        case COMMISSIONED:
            return new CommissionedEmployee(r);
        case HOURLY:
            return newHourlyEmployee(r);
        case SALARIED:
            return new SalaliedEmployee(r);
        default:
            throw new InvalidEmployeeType(r.type);
    }
  }
}

Use Descriptive Names

読み手の期待通りの動作をするように名前をつける
名前が長いことを恐れるな。謎の短い名前より長い名前の方がずっといい。
名前を考えることに時間を費やすことを恐れるな
名前の付け方は、統一させよう。

Function Arguments

引数の数は0~2に抑える。3は可能な限り避ける。4以上は基本ないと考えよう。
テスト観点でも3つ以上の引数を考慮するのは大変でしょう？
引数はなし or 単項であることが理想。
2項であることは理由があればOK（Point p = new Point(0, 0);など）

Flag Arguments

flag、ダメ絶対。
flagをつけることで、確実にfunctionは1つ以上のことをすることになるし、何よりもmethodを複雑化する要因になる。
render(boolean isSuite)とするくらいなら、renderForSuite()とrenderForSingleTest()に分けるべき。

Verbs and Keywords

write(name)よりはwriteField(name)
assertEquals(expected, actual)よりはassertExpectedEqualsActual(expected, actual)

Have No Side Effects

functionの名前から期待される動作とは別に副作用を持っているコードは撤廃しよう。障害につながりやすい。

public class UserValidator {
    private Cryptographer cryptographer;
    
    public boolean checkPassword(String userName, String password) {
        User user = UserGateway.findByName(userName);
        if (user != User.Null) {
            String codedPhrase = user.getPhraseEncodedByPassword();
            String phrase = cryptographer.decrypt(codedPhrase, password);
            if ("Valid Password".equals(phrase)) {
                Session.initialize();
                return true;
            }
        }
    }
}

Output Arguments

可読性を考慮すると、Output Argumentsはない方がいい。
appendFooter(s)よりはreport.appendFooter()

Commnad Query Separation

functionはオブジェクトの状態を変更する or オブジェクトの情報をreturnする。両方はやらない。

Prefer Exceptions to Returning Error Codes

if (deletePage(Page) == E_OK) {
    ...
} else {
    ...
}

try {
    deletePage(Page);
    ...
} catch (Exception e) {
    ...
}

Extract Try/Catch Blocks

try…catch文は分けた方が読みやすい、というだけの話。

public void delete(Page page) {
    try {
        deletePageAndAllReferences();
    }
    catch(Exception e) {
        ...
    }
}

private void deletePageAndAllReferences(Page page) throws Exception {
    ...
}

Don’t Repeat Yourself

同じコードを2箇所に書かない。

Structured Programming

inputとoutputは1つだけ、が望ましい。
できる限りreturnは複数書かない。
break, continue, gotoなどもできるだけ書かない。