段階を踏みつつリファクタリングしてみる


なんか一度こういうのやってみたかった

## 前提
+ `csv.py`という csv っぽい`list`をよしなにするモジュールを作ってみた、というところから
+ このモジュールは生まれたてで、まだまだ再設計する時間とリソースの余裕がある
+ ファイル入出力は面倒なので`f.py`というモジュールで適当に済ます
  + 本当は`path`とかも必要だけど、そこも適当
+ 言語はなんでも良かったんだけど、最近書いてないなーとふと思ったので python にした
+ お題と言語はあくまでイメージであり、お題自体に対する突っ込みやイケてる python かどうか等は興味がない

## 初版
### コード
```python:f.py
def write(lines):
    print 'write:', lines
```

```python:csv.py
# -*- coding: utf-8 -*-

import f


def as_kvs(rows):
    keys = rows[0]
    value_rows = rows[1:]

    return [dict(zip(keys, value_row)) for value_row in value_rows]


def write_first_one_or_empty(rows, sort_key, filter_key, filter_value, write_key):
    # 複数の kv にする
    kvs = as_kvs(rows)

    # 指定のキーでソートする
    _sorted = sorted(kvs, key=lambda row: row[sort_key])

    # 指定のキーが指定の値の箇所だけにフィルターする
    _filtered = filter(lambda row: row[filter_key] == filter_value, _sorted)

    if 0 < len(_filtered):
        # 先頭行を指定のキーで抽出する
        _mapped_one = map(lambda row: row[write_key], _filtered)[0]
        # 書き込む
        f.write([_mapped_one])
    else:
        # 空ファイル作成
        f.write([])


def write_all_or_empty(rows, filter_key, filter_value, write_key):
    # 複数の kv にする
    kvs = as_kvs(rows)

    # 指定のキーが指定の値の箇所だけにフィルターする
    _filtered = filter(lambda row: row[filter_key] == filter_value, kvs)

    if _filtered:
        # 全行を指定のキーで抽出する
        _mapped = map(lambda row: row[write_key], _filtered)
        # 書き込む
        f.write(_mapped)
    else:
        # 空ファイル作成
        f.write([])


def write_all_or_error(rows, filter_key, filter_value, write_key):
    # 複数の kv にする
    kvs = as_kvs(rows)

    # 指定のキーが指定の値の箇所だけにフィルターする
    _filtered = filter(lambda row: row[filter_key] == filter_value, kvs)

    if _filtered:
        # 全行を指定のキーで抽出する
        _mapped = map(lambda row: row[write_key], _filtered)
        # 書き込む
        f.write(_mapped)
    else:
        # エラー
        raise Exception("no result")
```

### 呼び元
```python:main.py
# status, code からなる csv を与え
# code でソートして
# status が active の最初の一件の
# code を書き出す
# なければファイル作成だけする

csv.write_first_one_or_empty(
    [['status', 'code'], ['dead', '001'], ['active', '003'], ['active', '002']],
    'code', 'status', 'active', 'code'
)

# 結果
# write: ['002']
```

```python:main.py
# name, gender からなる csv を与え
# gender が male の全件の
# name を書き出す
# なければファイル作成だけする

csv.write_all_or_empty(
    [['name', 'gender'], ['Eva', 'female'], ['Sunny', 'female']],
    'gender', 'male', 'name'
)

# 結果
# write: []
```

```python:main.py
# status, tel からなる csv を与え
# status が dead な全件の
# tel を書き出す
# なければエラー

csv.write_all_or_error(
    [['status', 'tel'], ['dead', '090-1111-1111'], ['active', '090-9999-9999']],
    'status', 'dead', 'tel'
)

# 結果
# write: ['090-1111-1111']
```


### 問題
+ 読みづらい
+ 共通化されていない
  + 今後に他メソッドを足す時に全コピーになってしまう
  + 同じコードの部分に修正が入ったら、何カ所直さなければいけないか分からない
+ コメントから得られる情報がない
  + コードを読めば分かることしか書いてない

## step-1 とりあえずコメントを英語で書いてみる
### 差分
 ```diff
 def write_first_one_or_empty(rows, sort_key, filter_key, filter_value, write_key):
-    # 複数の kv にする
+    # as kvs
     kvs = as_kvs(rows)

-    # 指定のキーでソートする
+    # sort by specified key
     _sorted = sorted(kvs, key=lambda row: row[sort_key])

-    # 指定のキーが指定の値の箇所だけにフィルターする
+    # filter by specified key and value
     _filtered = filter(lambda row: row[filter_key] == filter_value, _sorted)

     if 0 < len(_filtered):
-        # 先頭行を指定のキーで抽出する
+        # extract by specified key and first one
         _mapped_one = map(lambda row: row[write_key], _filtered)[0]
-        # 書き込む
+        # write
         f.write([_mapped_one])
     else:
-        # 空ファイル作成
+        # write empty
         f.write([])
``` 

```diff
 def write_all_or_empty(rows, filter_key, filter_value, write_key):
-    # 複数の kv にする
+    # as kvs
     kvs = as_kvs(rows)

-    # 指定のキーが指定の値の箇所だけにフィルターする
+    # filter by specified key and value
     _filtered = filter(lambda row: row[filter_key] == filter_value, kvs)

     if _filtered:
-        # 全行を指定のキーで抽出する
+        # extract by specified key
         _mapped = map(lambda row: row[write_key], _filtered)
-        # 書き込む
+        # write
         f.write(_mapped)
     else:
-        # 空ファイル作成
+        # write empty
         f.write([])
```

```diff
 def write_all_or_error(rows, filter_key, filter_value, write_key):
-    # 複数の kv にする
+    # as kvs
     kvs = as_kvs(rows)

-    # 指定のキーが指定の値の箇所だけにフィルターする
+    # filter by specified key and value
     _filtered = filter(lambda row: row[filter_key] == filter_value, kvs)

     if _filtered:
-        # 全行を指定のキーで抽出する
+        # extract by specified key
         _mapped = map(lambda row: row[write_key], _filtered)
-        # 書き込む
+        # write
         f.write(_mapped)
     else:
-        # エラー
+        # error
         raise Exception("no result")
 ```

### 次の問題
+ `# as kvs`とか`# write`とかあほくさいコメントがある
+ 情報量が変わっていないのに記載が多いのは二重管理でイケてない

## step-2 英語コメントを元にコメント整理とメソッド切り出しをする
### 方針
+ 英語コメントにしたことでコードとほぼ一致した箇所は、無価値コメントなので削除
+ そうでないコメントは、メソッド切り出しをしてメソッド名として使って削除

### 差分
切り出されて生まれたメソッドはこう

```diff 
+def sort_by_specified_key(kvs, key):
+    return sorted(kvs, key=lambda row: row[key])
```

```diff
+def filter_by_specified_key_and_value(kvs, key, value):
+    return filter(lambda row: row[key] == value, kvs)
```

```diff
+def extract_by_specified_key_and_first_one(kvs, key):
+    return kvs[0][key]

```

```diff
+def extract_by_specified_key(kvs, key):
+    return map(lambda row: row[key], kvs)
```

```diff
+def error():
+    raise Exception("no result")
```

本体部分はこうなった

```diff 
-def write_first_one_or_empty(rows, sort_key, filter_key, filter_value, write_key):
+def write_first_one_or_empty(rows, sort_key, filter_key, filter_value, extraction_key):
-    # as kvs
     kvs = as_kvs(rows)

-    # sort by specified key
-    _sorted = sorted(kvs, key=lambda row: row[sort_key])
+    _sorted = sort_by_specified_key(kvs, sort_key)

-    # filter by specified key and value
-    _filtered = filter(lambda row: row[filter_key] == filter_value, _sorted)
+    _filtered = filter_by_specified_key_and_value(_sorted, filter_key, filter_value)

     if 0 < len(_filtered):
-        # extract by specified key and first one
-        _mapped_one = map(lambda row: row[write_key], _filtered)[0]
-        # write
-        f.write([_mapped_one])
+        extracted_one = extract_by_specified_key_and_first_one(_filtered, extraction_key)
+        f.write([extracted_one])
     else:
-        # write empty
         f.write([])
```

```diff
-def write_all_or_empty(rows, filter_key, filter_value, write_key):
+def write_all_or_empty(rows, filter_key, filter_value, extraction_key):
-    # as kvs
     kvs = as_kvs(rows)

-    # filter by specified key and value
-    _filtered = filter(lambda row: row[filter_key] == filter_value, kvs)
+    _filtered = filter_by_specified_key_and_value(kvs, filter_key, filter_value)

     if _filtered:
-        # extract by specified key
-        _mapped = map(lambda row: row[write_key], _filtered)
-        # write
-        f.write(_mapped)
+        extracted = extract_by_specified_key(_filtered, extraction_key)
+        f.write(extracted)
     else:
-        # write empty
         f.write([])
```

```diff
-def write_all_or_error(rows, filter_key, filter_value, write_key):
+def write_all_or_error(rows, filter_key, filter_value, extraction_key):
-    # as kvs
     kvs = as_kvs(rows)

-    # filter by specified key and value
-    _filtered = filter(lambda row: row[filter_key] == filter_value, kvs)
+    _filtered = filter_by_specified_key_and_value(kvs, filter_key, filter_value)

     if _filtered:
-        # extract by specified key
-        _mapped = map(lambda row: row[write_key], _filtered)
-        # write
-        f.write(_mapped)
+        extracted = extract_by_specified_key(_filtered, extraction_key)
+        f.write(extracted)
     else:
-        # error
-        raise Exception("no result")
+        error()
```

### 次の問題
+ `filter_key`と`filter_value`を`==`することしかできないので、例えば`age < 20`の様なフィルターが不可能
+ リストへのインデックスアクセスが頻出している

## step-3 少し局所リファクタリングをする
### 差分
インデックスアクセスでは少し不親切なので`head`と`tail`を用意した

```diff
+def head(xs):
+    return xs[0]
```

```diff
+def tail(xs):
+    return xs[1:]
```

利用箇所

```diff
 def as_kvs(rows):
-    keys = rows[0]
-    value_rows = rows[1:]
+    keys = head(rows)
+    value_rows = tail(rows)
```

```diff
 def extract_by_specified_key_and_first_one(kvs, key):
-    return kvs[0][key]
+    return head(kvs)[key]
```

`filter_by_specified_key_and_value`はもう少し融通が利くように`key, value`ではなく`predicate`を受け取るように変更

```diff
-def filter_by_specified_key_and_value(kvs, key, value):
-    return filter(lambda row: row[key] == value, kvs)
+def filter_by_predicate(kvs, predicate):
+    return filter(predicate, kvs)
```

そうすると`filter_by_predicate`は`filter`以上の事を何もしていないので破棄してしまう

```diff
-def filter_by_predicate(kvs, predicate):
-    return filter(predicate, kvs)
```

それから`head`を作ったので`extract_by_specified_key_and_first_one`もわざわざ用意してあげなくても良いかもしれない
小さく分けたメソッドの名前がちゃんとしていれば、組み合わせて使っても処理が不明瞭にはなりづらい

```diff
-def extract_by_specified_key_and_first_one(kvs, key):
-    return head(kvs)[key]
```

ここまでを反映した本体部分はこう

```diff
-def write_first_one_or_empty(rows, sort_key, filter_key, filter_value, extraction_key):
+def write_first_one_or_empty(rows, sort_key, predicate, extraction_key):
     kvs = as_kvs(rows)

     _sorted = sort_by_specified_key(kvs, sort_key)

-    _filtered = filter_by_specified_key_and_value(_sorted, filter_key, filter_value)
+    _filtered = filter(predicate, _sorted)

     if 0 < len(_filtered):
-        extracted_one = extract_by_specified_key_and_first_one(_filtered, extraction_key)
+        extracted_one = head(_filtered)[extraction_key]
         f.write([extracted_one])
     else:
         f.write([])
``` 

```diff
-def write_all_or_empty(rows, filter_key, filter_value, extraction_key):
+def write_all_or_empty(rows, predicate, extraction_key):
     kvs = as_kvs(rows)

-    _filtered = filter_by_specified_key_and_value(kvs, filter_key, filter_value)
+    _filtered = filter(predicate, kvs)

     if _filtered:
         extracted = extract_by_specified_key(_filtered, extraction_key)
         f.write(extracted)
     else:
         f.write([])
``` 

```diff
-def write_all_or_error(rows, filter_key, filter_value, extraction_key):
+def write_all_or_error(rows, predicate, extraction_key):
     kvs = as_kvs(rows)

-    _filtered = filter_by_specified_key_and_value(kvs, filter_key, filter_value)
+    _filtered = filter(predicate, kvs)

     if _filtered:
         extracted = extract_by_specified_key(_filtered, extraction_key)
         f.write(extracted)
     else:
         error()
``` 

### 次の問題
+ 書き込むか異常ハンドルかの部分にやや重複コードがある
+ 処理が似ているので共通化が出来そう

## step-4 コードの見た目が似ている箇所を共通化する（悪い方針）
### 方針
+ ３つある本体メソッドの下２つ、その中の`filter`から`f.write`までのコードが完全に同じなので、そこを切り出す

### 差分
```diff
+def filter_and_extract_and_write_if_not_empty(kvs, predicate, extraction_key):
+    _filtered = filter(predicate, kvs)
+
+    if _filtered:
+        extracted = extract_by_specified_key(_filtered, extraction_key)
+        f.write(extracted)
```

切り出されたので本体の行数が減った

```diff
 def write_all_or_empty(rows, predicate, extraction_key):
     kvs = as_kvs(rows)

-    _filtered = filter(predicate, kvs)
+    filter_and_extract_and_write_if_not_empty(kvs, predicate, extraction_key)

-    if _filtered:
-        extracted = extract_by_specified_key(_filtered, extraction_key)
-        f.write(extracted)
-    else:
+    if not kvs:
         f.write([])
```

```diff
 def write_all_or_error(rows, predicate, extraction_key):
     kvs = as_kvs(rows)

-    _filtered = filter(predicate, kvs)
+    filter_and_extract_and_write_if_not_empty(kvs, predicate, extraction_key)

-    if _filtered:
-        extracted = extract_by_specified_key(_filtered, extraction_key)
-        f.write(extracted)
-    else:
+    if not kvs:
         error()
```

### この方針の問題
+ kvs は異常ハンドルに必要なので共通化出来ない
+ 切り出された`filter_and_extract_and_write_if_not_empty`が流用しづらい
  + 今後４つめの本体メソッドが出来たとして、これにぴったり合致する処理が必要になるか？
+ メソッド名が長い
  + 単語が多く、`and`が使われている
+ `and`を持つ場合は、やってくれることが多い
  + やってくれることが多いと、理解が難しくなる
  + やってくれることが多いと、再利用時に用途が合致しづらくなる
+ `if`の方だけ切り出したので、`else`部は本体側に残った
  + 切り出された処理の中を全部読まないと使い方も異常ハンドルをどうしたら良いかも分からない

### 何が悪かったのか  
+ 本来の処理の流れはこうだった
  + データ構造変換
  + 抽出
  + 加工
  + 書き出し or 異常ハンドル
+ それをこうまとめてしまった
  + データ構造変換
  + 抽出・加工・書き出し
  + or 異常ハンドル
+ コードが似ているという理由で切り出しをしてはならず、処理の流れの単位で切り出さないとうまくいかない

## step-4 処理の流れを整理する（良い方針）
先述の悪い方針は切り戻してやりなおす

### 方針
+ 似たコードではなく、リストを書き込む箇所の流れに注目する

### 差分
本体メソッドの２つ目に注目する

今回はリストの中身がある場合と空リストの場合で処理を明示的に分ける必要はない
基本的にリストを渡す先の処理が同じなら、リストの長さを意識する必要は無い（してはいけない）

ここは中身があるか空リストかを知らぬまま`f.write`に渡してしまう方が良い

```diff
 def write_all_or_empty(rows, predicate, extraction_key):
     kvs = as_kvs(rows)

     _filtered = filter(predicate, kvs)

-    if _filtered:
-        extracted = extract_by_specified_key(_filtered, extraction_key)
-        f.write(extracted)
-    else:
-        f.write([])
+    extracted = extract_by_specified_key(_filtered, extraction_key)
+    f.write(extracted)
```

次に本体メソッドの１つ目にも注目する

こちらは空リストでは無い場合に先頭要素を取り出して加工してまたリスト化している

この

+ 先頭要素を取り出す
+ 単一要素の加工
+ 再リスト化

という流れを

+ 長さが最大 1 のリストで保持する
+ `map`で全件（１つだが）加工する

という流れに発想を変えてみる

まずはリストから合致する先頭要素を最大長さ 1 で得るメソッドを用意する

```diff
+def find_first(kvs, predicate):
+    for kv in kvs:
+        if predicate(kv):
+            return [kv]
+    else:
+        return []
```

そうしてしまえば先ほどの本体メソッドの２つ目と同様に中身があるかどうかを意識する必要はなくなる
しかもリスト変換なので`head`と`key`アクセスではなく`extract_by_specified_key`も使える

```diff
 def write_first_one_or_empty(rows, sort_key, predicate, extraction_key):
     kvs = as_kvs(rows)

     _sorted = sort_by_specified_key(kvs, sort_key)
+    first = find_first(_sorted, predicate)

-    _filtered = filter(predicate, _sorted)
-
-    if 0 < len(_filtered):
-        extracted_one = head(extract_by_specified_key(_filtered, extraction_key))
-        f.write([extracted_one])
-    else:
-        f.write([])
+    extracted = extract_by_specified_key(first, extraction_key)
+    f.write(extracted)
```

本体メソッド１と２の`if`節も`else`節も「リスト長を意識しない書き出し処理」という粒度に収まった
悪い例では`if`と`else`は一連の書き出し処理の仲間だったのに、`if`の方だけを切り出してしまっていたことが悪手だった

### 次の問題
+ アクセス修飾子を気にしていない
+ `csv`というモジュールだがただのリスト操作の処理がある

## step-5 切り出したメソッドの整理をする
### 方針
+ 切り出したそれぞれが`public`か`private`かを整理する

### 差分
+ `head`や`tail`は今後別のモジュールでも使いたくなりそうだから`public`
  + もっと考えると、こいつらはリスト操作しかしていないので`l.py`を作って`csv.py`から追い出す
  + （`list.py`でも良かったけど、標準の`list()`と名前が競合するので適当にずらした）

```diff:l.py
+# -*- coding: utf-8 -*-
+
+
+def head(xs):
+    return xs[0]
+
+
+def tail(xs):
+    return xs[1:]
+
+
+def find_first(xs, predicate):
+    for x in xs:
+        if predicate(x):
+            return [x]
+    else:
+        return []
```

+ よく見ると`kvs`を操作してソートしたり抽出したりもただの辞書操作なので、`d.py`を作って`csv.py`から追い出す
  + その際に変数名も`kvs`から抽象的な適当な名前に変える

```diff:d.py
+# -*- coding: utf-8 -*-
+
+
+def extract_by_specified_key(xs, key):
+    return map(lambda x: x[key], xs)
+
+
+def sort_by_specified_key(xs, key):
+    return sorted(xs, key=lambda x: x[key])
```

+ 残った処理は`csv.py`のためだけにあるメソッドなので`private`化する

```diff
-def as_kvs(rows):
+def __as_kvs(rows):
```

```diff
-def error():
+def __error():
```

### なぜ一見便利な `as_kvs` を切り出さないか
`kvs`は`csv.py`の期待するデータ構造を内部で処理しやすくするためだけに変換して用意した一時的なデータ構造に過ぎない

`kvs`は実際には`dict`だが、`csv.py`の公開している引数にも戻り値にも`dict`型は現れていない
つまりモジュールの提供する機能としてはどこにも`dict`型は存在しないので、公開するべきではない

加えて`as_kvs`の引数の型は`list`だが、`header + [body]`構成であることや全てのカラムが一致している事等の制約があるため、`l.py`や`d.py`よりは`csv.py`の非公開処理とする方が望ましいと考えた

### 次の問題
+ `private`メソッドが何カ所から使われているか分かりづらい
  + （はずだったんだけど、案外他モジュールに出て行ってしまった...）

## step-6 一度しか使われていない `private` メソッドをリファクタリングする（ここは賛否両論かも）
やや大きなモジュールになると、沢山の`private`がそれぞれどこからどんな風に使われているかが極めて把握しづらくなることがある

そうすると`private`メソッドのリファクタリングがしづらかったり影響範囲が分かりづらかったりして結構大変な目に遭うことがある

### 沢山 `private` があるけどそれぞれ１度しか使われていない様な例
```
public 1 -> private 1 -> private 2 -> private 3
public a -> private a -> private b
public x -> private x
```

これは実質３つの`public`が存在すると捉えた方が全体像の把握がはるかに容易だったりする
`private`も割と安心してリファクタリング出来たりする

### １度しか使われない `private` と色んな箇所に依存している `private` が混じっている例
```
public 1 -> private 1
            |
            V
public a -> private a -> private b
                         ^
                         |
public x -> private x -> private y
```

`private 1`や`private x`はある程度簡単に変更出来るが、`private b`を軽い気持ちで直すと全ての`public`に影響する

### 方針
１箇所からしか使われない`private`を、メソッドの中に定義してみる

これは僕があるとき勝手にやってみた方法でちゃんとした出展や議論はないけど、割と良手だと思って使い捨てコードやソロプロジェクトでは多用している

### 差分
`__error`は一箇所でしか使っていないので、`def`の中に`def`で用意してみた

ついでにそれに伴い、どうせ外からは見えなくなったので目障りな`__`を削除した

```diff
 def write_all_or_error(rows, predicate, extraction_key):
+    def write_or_error(kvs):
+        if kvs:
+            f.write(kvs)
+        else:
+            raise Exception("no result")
+
     kvs = __as_kvs(rows)

     _filtered = filter(predicate, kvs)

     extracted = d.extract_by_specified_key(_filtered, extraction_key)
-    if extracted:
-        f.write(extracted)
-    else:
-        __error()
+    write_or_error(extracted)
```

`__as_kvs`は当然何カ所からも使われているので変更しない

### 最後の問題
+ テストを書いてない
+ このままだと file-io が頻出するのでテストが書きづらい
  + file-io ならテストは不可能ではないが、標準出力やメール送信の様な処理だと本当にテスト出来なかったりする

## step-7 変換と書き出しを分離してテストを書く
### 方針
+ 処理の流れを大きく捉えると「変換」と「出力」に分けられる
+ このうちしっかり動作確認をしたいのは「変換」の方
+ 大抵の場合「出力」はテストしづらい

なのでモジュールの責任範囲を「変換」だけにして、テストを実装する

### 差分
「出力」はせずに`return`して終わることとする
それに伴いメソッド名も`write_`から`extract_`に変更した

```diff 
-def write_first_one_or_empty(rows, sort_key, predicate, extraction_key):
+def extract_first_one_or_empty(rows, sort_key, predicate, extraction_key):
     kvs = __as_kvs(rows)

     _sorted = d.sort_by_specified_key(kvs, sort_key)
     first = l.find_first(_sorted, predicate)

-    extracted = d.extract_by_specified_key(first, extraction_key)
-    f.write(extracted)
+    return d.extract_by_specified_key(first, extraction_key)
``` 

```diff 
-def write_all_or_empty(rows, predicate, extraction_key):
+def extract_all_or_empty(rows, predicate, extraction_key):
     kvs = __as_kvs(rows)

     _filtered = filter(predicate, kvs)

-    extracted = d.extract_by_specified_key(_filtered, extraction_key)
-    f.write(extracted)
+    return d.extract_by_specified_key(_filtered, extraction_key)
``` 

```diff 
-def write_all_or_error(rows, predicate, extraction_key):
-    def write_or_error(kvs):
-        if kvs:
-            f.write(kvs)
+def extract_all_or_error(rows, predicate, extraction_key):
+    def it_or_error(xs):
+        if xs:
+            return xs
         else:
             raise Exception("no result")

     kvs = __as_kvs(rows)

     _filtered = filter(predicate, kvs)

     extracted = d.extract_by_specified_key(_filtered, extraction_key)
-    write_or_error(extracted)
+    return it_or_error(extracted)
```

結果`import`が消えるので、`csv.py`は file-io の処理はしなくなったことが確認できた

```diff
-from private import f
```

テストはこんなイメージ

```python:csv_test.py
# -*- coding: utf-8 -*-

import csv

assert csv.extract_first_one_or_empty(
    [['status', 'code'], ['dead', '001'], ['active', '003'], ['active', '002']],
    'code', lambda kvs: kvs['status'] == 'active', 'code'
) == ['002']

assert csv.extract_first_one_or_empty(
    [['status', 'code'], ['dead', '001']],
    'code', lambda kvs: kvs['status'] == 'active', 'code'
) == []
```

## おしまい
### 完成形
```python:csv.py
# -*- coding: utf-8 -*-

import l
import d


def __as_kvs(rows):
    keys = l.head(rows)
    value_rows = l.tail(rows)

    return [dict(zip(keys, value_row)) for value_row in value_rows]


def extract_first_one_or_empty(rows, sort_key, predicate, extraction_key):
    kvs = __as_kvs(rows)

    _sorted = d.sort_by_specified_key(kvs, sort_key)
    first = l.find_first(_sorted, predicate)

    return d.extract_by_specified_key(first, extraction_key)


def extract_all_or_empty(rows, predicate, extraction_key):
    kvs = __as_kvs(rows)

    _filtered = filter(predicate, kvs)

    return d.extract_by_specified_key(_filtered, extraction_key)


def extract_all_or_error(rows, predicate, extraction_key):
    def it_or_error(xs):
        if xs:
            return xs
        else:
            raise Exception("no result")

    kvs = __as_kvs(rows)

    _filtered = filter(predicate, kvs)

    extracted = d.extract_by_specified_key(_filtered, extraction_key)
    return it_or_error(extracted)
```

### 初版と比べて
+ コメントは一切なくなったが、メソッド名をちゃんと付けたので情報量は減っていない
+ `if`や`for`の様な所謂システム処理がなく、全部メソッド呼び出しで済んでいる
  + これにより英文のような感覚で流れを把握することが出来る
  + （内部`def`がある場合はそこを読み飛ばして本文開始点から読む）
+ 今後も役に立ちそうな`l.py`と`d.py`が生まれた
+ ただの「変換」になり戻りがシンプルな`list`型になったので、呼び元はこの後さらなる加工を行ったりすることが出来る
+ 戻りで得られたリストは別の変換が続けられるだけでなく、ログに出したりレポート作成に組み込んだりも自由に出来る
+ テストが書ける / ある
+ 手間はかかったが、実は行数が減っている
  + これは双方から空行を消してさらに初版の方からコメントを消しても、もともとコメントのない完成品の方が行数が少ない

## 締め
### 要点
要点と言い切るほど端的に言えてない...

#### 価値のないコメントは書かない
+ コードをただ日本語化している様なコメント
  + `return result # 結果を返却`の様なヤツ、案外良くある
+ 慣れると要らないし、そもそも要らない
  + おとなしくコードを読もう
+ 同じ事をコードとコメントで書くと言うことは、修正箇所を倍増させている
  + そしてコメントはどう書いても実行に影響しないので、正しく直される保証がない
+ 大抵の場合「手段」が書かれている

#### じゃあ逆に書くべきコメントとは？
+ なぜ
  + `# この処理は〜〜の仕様のためにあります`、とか
  + `# 〜〜を考慮してこういう実装にしました`、とか
  + `# 〜〜は〜〜によりあり得ないので例外とします`、とか
+ 仕様書のパスや言語やフレームワークのバグ issue の url 、とか
+ その他基本的にあとで誰かがコードを読むだけでは理解できない付加情報をコメントする
+ 大抵の場合「目的」が書かれている

#### そうは言ってもコメントがないと辛い、と思った場合
+ コードが悪くてそう思う場合は、今回の例のようにコメントではなく`private`化して名前をちゃんと付ける
  + コメントは間違っても動くけど、コードは間違えられない
  + 再利用も出来る
+ スキル不足で単純にコードが読めなかったり日本語がないと不安な場合は、脱コメントを訓練だと思って頑張って力量を上げよう
  + 厳しいようだけど、そのレベルにチーム活動のレベルを下げることは出来ない
  + よほど異様に難しいとか魔術とかは別だけど、大抵の業務システムのコーディングなんてそんなに難しいことはしていない

#### 悪そうな `private` メソッド
+ メソッド名に`and`とか入ってるとかなり疑わしい
  + やることが多いメソッドはそれだけ理解しづらい
  + やることが多いほど使える状況が特化していくので再利用しづらい
+ 実装を読まないと使い方が分からない
  + これとこれを引数で渡すと、こうしてああしてくれるのね？うんうん、って中の奥まで読む事、案外ない？
  + 読まないと使えないなら、実質的には行数は減っていない

#### 良い `private` メソッド
+ 末端の`private`メソッドは、名前がシンプルでやることも１つである方が良い
  + `len()`とか`head()`とか`.split()`とか
  + もちろん`private`を集約する facade の`private`はその限りではないけど
+ 中を読まなくても使える
  + いちいち`head()`の中身とか読もうとしないよね

#### `private` と `public` のメソッドの扱いの違い
+ 単にアクセス修飾子としてではなく、モジュールとして公開したいかで考える
+ そのメソッド単品で役に立てないような処理は公開してはいけない
+ たくさんの小さい`public`を用意する方が、後々どんどん楽になる
+ 逆に整備されていないモジュールを増やしてしまうほど、後々どんどん辛くなる

#### 何をテストするか
+ 「入力」や「出力」ではなくて、「変換」に狙いを定めて入念にテストする
+ `public`な公開されているメソッドはちゃんとテストする
  + それにより安心して組み合わせられる

#### コードの見た目が似ているという理由で共通化しない
+ 先に処理の流れを整理して、やりたいことを抽象的に整理する
  + そのやりたいことが似ている箇所を共通化する
+ 本来やりたいことが違うのにたまたま一部だけ処理（手段）が同じ様なことがあるが、それを安易に切り出さない
  + `filter`を`find_first`にした箇所と最後まで`filter`だった箇所
  + `if write else empty`と`if write else error`の`if`部

#### `def` in `def`
+ コメントをメソッド化して生まれた`private`メソッドが一箇所からしか呼ばれていない場合のテクニック
+ 有効なスコープが狭いほど理解の難易度が下がり影響範囲が狭くなる
+ 多分大抵の言語で出来る

scala は素直に出来る

```scala
def upperJoin(xs: List[String], sep: String): String = {
    def upper(xs: List[String]): List[String] = xs.map(_.toUpperCase)
    def join(xs: List[String]): String = xs.mkString(sep)

    join(upper(xs))
}

upperJoin(List("hello", "world"), " ") // HELLO WORLD
```

java でも`Function<T, R>`とかを使えば出来なくはない

```java
public static String upperJoin(List<String> xs, String sep) {
    Function<List<String>, List<String>> upper = _xs -> _xs.stream().map(String::toUpperCase).collect(toList());
    Function<List<String>, String> join = _xs -> _xs.stream().collect(joining(sep));

    return join.apply(upper.apply(xs));
}

upperJoin(asList("hello", "world"), " "); // HELLO WORLD
```

haskell も出来る
というか発想が haskell の関数を値で定義する感覚から来ている

```haskell
upperJoin xs sep = (join . upper) xs
  where
    upper = map (map toUpper)
    join = intercalate sep

upperJoin ["hello", "world"] " " -- HELLO WORLD
```

なので java の例は haskell に近いし、python でこう書くことも出来る
（PEP 8 では`lambda`に名前を付けて束縛するのはコーディング規範違反だけど、ネストが嫌なので一人だとこっちの方が多い）

```python
def upper_join(xs, sep):
    upper = lambda xs: [x.upper() for x in xs]
    join = lambda xs: sep.join(xs)

    return join(upper(xs))

upper_join(['hello', 'world'], ' ') # HELLO WORLD
```

### 反省  
+ 教科書的にはテストがあるところからリファクタリングするんだけど、そこをサボった
  + 初版に「出力」を入れたかったのでテストがない状態から始めた
  + まーこれくらいの規模なら間違っても気づけると思うし...
  + そもそも完成後にさっとテスト書くくらいの方が僕は好き
+ `kvs`は`dict`の`list`だったので、安易に`d.py`に出したのはちょっと違った...
  + ま、そもそも例は雰囲気のためにしか存在しないので、いーけど

### おしまい
やってみたかっただけなので、満足した

どうでも良いけど、赤地と緑地に黒い字ってのを見てるといっつも「スイカみたいだな〜」って思う