27か月前公開・27か月前更新・0 pv・8 min read

リーダブルコードの演習課題を作ってみる④

https://cdn.magicode.io/media/notebox/a6dd784a-ea9e-45c4-a31f-d4433fa4cbce.jpeg

はじめに

後輩のプログラミング教育をしていて、リーダブルコード※を読んでもらっても、読んだだけでは定着していなかった。
コードレビューで定着を試みているが、今後も後輩が増えていくことを考えて、何か対応策を講じようと思い、試しに演習課題を作ってみることにした。

リーダブルコードの2章以降から、気になったテクニックに対して、演習課題を作っていく。

今回は、「3章誤解されない名前」を題材にする。

なお、プログラミング言語はpythonとする。

※ https://www.oreilly.co.jp/books/9784873115658/

書籍の内容

3章全体で「誤解される名前に気をつけろ」をテーマに解説している。鍵となる考えは

名前が「ほかの意味と間違えられることはないだろうか？」と何度も自問自答する。

である。

ここでは本に書かれていた内容の一部を紹介する。

fillter()

下記のようなDatabase問い合わせ処理があったとする。

results = Database.all_objects.filter("year <= 2011")

この場合、「year <= 2011」のデータと「year <= 2011」ではないデータのどちらが返ってくるのだろうか？
filterという単語があいまいで、「選択する」のか「除外する」のかわからず、誤解を招いている。
「選択する」の場合は、select()、「除外する」の場合はexclude()にするとよい。

境界条件関連

off-by-one エラーと呼ばれる境界条件の判定に関するエラーがある。
これは、ループ回数の指定を間違えて、正しい回数より1回多かったり少なく実行されたときに発生する。典型例としては、数値比較の際に「～未満( x < N)」とするべきところを「～以下(x <= N)」としてしまうなどがある。

例: リーダブルコードより

CART_TOO_BIG_LIMIT = 10

if shopping_cart.num_items() >= CART_TOO_BIG_LIMIT:
    Error("カートにある商品が多すぎます")

CART_TOO_BIG_LIMIT があいまいなため、未満なのか以下なのかわからない。ショッピングカートには10個まで商品を入れられるようにしたい場合、エラーになるが条件文の正誤が分かりにくい。このように限界値を明確にしたい場合は、min, maxを名前の前につける と良い。

他にも、範囲を指定するときはfirst, lastを使う や 包含/排他的範囲にはbegin, endを 使うとよい。
前者ではfirstから始まり、lastまで含み、意味上では先ほどのmin, maxと同等なので、読んでわかる方を使う。
後者は、beginは含み、endは含まないケースである。残念なことにendではあいまいな表現だが、英語に適した単語がないため、begin, endの対をイディオムとして用いている。

演習課題

いくつか例題を上げる。
鍵となる考えを意識して、どの名前をどうすべきかを考えてみてほしい。

名前が「ほかの意味と間違えられることはないだろうか？」と何度も自問自答する。

課題1

class Item:
    pass

class Items:
    def __init__(self, items: list[Item]):
        self._items = items

    def check(self, item: Item) -> bool:
        return items in self._items

課題2

class Human:
    def __init__(self, name: str, age: int, height: int):
        self._id: UUID = uuid4()
        self._name: str = name
        self._age: int = age
        self._height: int = height

    @property
    def name(self) -> str:
        return self._name
    
    @peroperty
    def age(self) -> int:
        return self._age
    
    @property
    def height(self) -> int:
         return self._height

    def compare(self, other: Human) -> bool:
        return self.age > other. age

課題3

UPPER_LIMIT: int = 50
LOWER_LIMIT: int = 0

class Item:
    pass

class Items:
    class ItemsLenghError(Exception):
        pass

    def __init__(self, items: list[Item]):
        self._items = items

    def append(self, item: Item):
        if len(self._items > UPPER_LIMIT):
            raise ItemsLengthError("アイテム数の上限を超えています")
        self._items.append(item)

    def remove(self, item: Item):
        if len(self._items < LOWER_LIMIT):
            raise ItemsLengthError("アイテム数の下限を超えています")
        self._items.remove(item)

課題4

class Widget:
    def reload(self):
        pass

widget: Widget = Widget()

# 一定期間ごとに呼ばれるとする
def update():
    if self.reload():
        widget.reload()

def reload() -> bool:
    # 再読み込みが必要かの判断
    return xxx

課題5

class Item:
    pass

class Items:
    def __init__(self, items: list[Item]):
        self._items = items

    def append(self, item: Item):
        self._items.insert(0, item)

解答例

課題1

check だと曖昧で何を確認したいのかわからない。追加してもよいかのバリデーションチェックかもしれないし、要素として持っていないことの確認かもしれない。

ここでは、引数で渡したインスタンスを要素として持っているかを確認している。なので、has_item(), contains(), include() などが考えられる。has_item() は、hasという接頭語でbool値が返ってくると予想できる点は良いた、itemが少し冗長にみえる。containsはC#, includeはrubyなど他言語で用いられる表現なので、他言語を扱った経験がある人にはなじみ深くていいだろう。

類似例として、find() も考えられるが、返り値としては、探した結果が返るのが望ましくboolを返すのは不適切だろう。そうなると、要素を持っているかどうかをユーザー側が判断しなければならなくなってしまう。

class Item:
    pass

class Items:
    def __init__(self, items: list[Item]):
        self._items = items

    # checkだと何を確認したいのかわからない。
    #def check(self, item: Item) -> bool:
    #    return items in self._items
    def contains(self, item: Item) -> bool:
        return items in self._items

課題2

compare() だと何を比較しているのかわからない。年齢？身長？体重？etc。さらに高いとき低いときどちらの時がtrueなのかわからない。

ここでは、年齢を比較して年齢が高い時にtrueとしている。older や bool値を強調するなら　is_old などが考えられる。

class Human:
    def __init__(self, name: str, age: int, height: int):
        self._id: UUID = uuid4()
        self._name: str = name
        self._age: int = age
        self._height: int = height

    @property
    def name(self) -> str:
        return self._name
    
    @peroperty
    def age(self) -> int:
        return self._age
    
    @property
    def height(self) -> int:
         return self._height

    #def compare(self, other: Human) -> bool:
    #    return self.age > other. age
    def older(self, other: Human) -> bool:
        return self.age > other. age

課題3

これは、書籍で記載されていたとおり、Max, Minを使おう。あと、名前が抽象的過ぎて、何の要素に影響するかがみえない。

# UPPER_LIMIT: int = 50
# LOWER_LIMIT: int = 0
MAX_ITEMS: int = 50
MIN_ITEMS: int = 0

class Item:
    pass

class Items:
    class ItemsLenghError(Exception):
        pass

    def __init__(self, items: list[Item]):
        self._items = items

    def append(self, item: Item):
        if len(self._items > MAX_ITEMS):
            raise ItemsLengthError("アイテム数の上限を超えています")
        self._items.append(item)

    def remove(self, item: Item):
        if len(self._items < MIN_ITEMS):
            raise ItemsLengthError("アイテム数の下限を超えています")
        self._items.remove(item)

課題4

reload() だと再読み込みを実行するように見えるし、実際Widgetではそうなっており混乱する。should_reload() あたりであれば、「リロードすべきか？」と伝わってくる、

class Widget:
    def reload(self):
        pass

widget: Widget = Widget()

# 一定期間ごとに呼ばれるとする
def update():
    if self.reload():
        widget.reload()

# def reload() -> bool:
#     # 再読み込みが必要かの判断
#     return xxx
def should_reload() -> bool:
    # 再読み込みが必要かの判断
    return xxx

課題5

append() だと末尾に追加するようにみえる。実際pythonのlist.append()はその挙動である。prepend() や add_fisrt() などが良いだろう。

class Item:
    pass

class Items:
    def __init__(self, items: list[Item]):
        self._items = items

    # def append(self, item: Item):
    #     self._items.insert(0, item)
    def add_first(self, item: Item):
        self._items.insert(0, item)