python3

Pythonでオブジェクト指向エクササイズをやってみた

元ネタ

オブジェクト指向エクササイズをやってみる

Pythonの勉強を始めて、ようやく一通りのコーディングを何も見ずにできるようになりました。
PEP8のコーディング規約を守ったり、Effective Pythonに書かれているようなことを真似たりして、
ある程度、まともなコードを書けるようになったつもりです。

ただ、設計となると、いまいち自信も無いし、メンターもいないので、
Pythonでオブジェクト指向の練習どうしたらいいんだろう、って思ってたら
オブジェクト指向エクササイズなるものを発見しました。

元ネタはJavaになっているのですが、
リファクタリング前のコードをまずはPythonに書き換えて、
オブジェクト指向エクササイズやってみました。

エクササイズで守らなければいけない9つのルールを
自分の判断で守らなかった(守れなかった)箇所もありますが、
Beforeよりは良い設計ができたのではないかと思います。

オブジェクト指向エクササイズ(Pythonバージョン)

Before
After

オブジェクト指向エクササイズとは

詳しいことは他の記事を見てもらった方が早いと思うので割愛します。

エクササイズでは以下のルールを守らなければいけません

  1. 1つのメソッドにつきインデントは1段階までにすること
  2. else 句を使用しないこと
  3. すべてのプリミティブ型と文字列型をラップすること
  4. 1行につきドットは1つまでにすること
  5. 名前を省略しないこと
  6. すべてのエンティティを小さくすること
  7. 1つのクラスにつきインスタンス変数は2つまでにすること
  8. ファーストクラスコレクションを使用すること
  9. Getter, Setter, プロパティを使用しないこと

オブジェクト指向エクササイズの実践

今回の例題は自動販売機のプログラムです。
エクササイズをしたい方はこの記事を見る前に
Beforeのコードを使って自分でエクササイズやってから、
Afterのコードと比較して、意見とかもらえたら嬉しいです。

大まかな変更箇所は以下の通りです。

お金はint型ではなくCoinというenumでクラス化

自動販売機におけるお金はコインで管理されるべきだと思ったので、
Coinというenum型のクラスを作り、CoinHolderでコインの枚数を管理するようにしました。

利点としては、例えば元の設計では、100円と500円しか考慮されておらず、
もし50円や10円を追加しようとすると、buyメソッドがとても長くなります。

enum型のCoinで管理すれば、CoinクラスとCoinHolderのコンストラクタだけを変更すれば良いので、変更が容易になります。

ファーストクラスコレクションを使用することが8のルールにありますが、
Pythonでは辞書型があるので、
enumで作ったコインをfor文で回して辞書に追加するという処理で
CoinHolderのほとんどの処理ができました。

Before

qiita.py
from drink import Drink


class VendingMachine:

    def __init__(self):
        self.quantity_of_coke = 5
        self.quantity_of_diet_coke = 5
        self.quantity_of_tea = 5
        self.number_of_100_yen = 10
        self.change = 0

    # 投入金額. 100円と500円のみ受け付ける.
    # return. ジュース or None
    def buy(self, i, kind_of_drink):

        if (i != 100) and (i != 500):
            self.change += i
            return None

        if (kind_of_drink == Drink.COKE) and (self.quantity_of_coke == 0):
            self.change += i
            return None
        elif (kind_of_drink == Drink.DIET_COKE) and \
                (self.quantity_of_diet_coke == 0):
            self.change += i
            return None
        elif (kind_of_drink == Drink.TEA) and (self.quantity_of_tea == 0):
            self.change += i
            return None

        if i == 500 and self.number_of_100_yen < 4:
            self.change += i
            return None

        if i == 100:
            self.number_of_100_yen += 1
        elif i == 500:
            self.change += (i - 100)
            self.number_of_100_yen -= (i - 100) / 100

        if kind_of_drink == Drink.COKE:
            self.quantity_of_coke -= 1
        elif kind_of_drink == Drink.DIET_COKE:
            self.quantity_of_diet_coke -= 1
        else:
            self.quantity_of_tea -= 1

        return Drink(kind_of_drink)

    def refund(self):
        result = self.change
        self.change = 0
        return result

After

coin.py
from enum import Enum


class Coin(Enum):
    YEN_500 = 500
    YEN_100 = 100
    YEN_10 = 10
coin_holder.py
from coin import Coin
import copy


class CoinHolder:

    def __init__(self, yen_500=0, yen_100=0, yen_10=0):
        self.coin_stocks = {}
        self.coin_stocks.update({Coin.YEN_500.name: yen_500})
        self.coin_stocks.update({Coin.YEN_100.name: yen_100})
        self.coin_stocks.update({Coin.YEN_10.name: yen_10})

    def __add__(self, other):
        new_holder = CoinHolder()

        for coin in Coin:
            new_holder.coin_stocks[coin.name] = \
                self.coin_stocks[coin.name] + other.coin_stocks[coin.name]

        return new_holder

    def __sub__(self, other):
        new_holder = CoinHolder()

        for coin in Coin:
            new_holder.coin_stocks[coin.name] = \
                self.coin_stocks[coin.name] - other.coin_stocks[coin.name]

        return new_holder

    def sum(self):
        sum_of_coins = 0

        for coin in Coin:
            sum_of_coins += self.coin_stocks[coin.name] * coin.value

        return sum_of_coins

    def reset(self):
        for coin in Coin:
            self.coin_stocks[coin.name] = 0
        return self

    def calc_coin(self, change: int):
        my_coin_holder = CoinHolder()
        val = change
        coin_stocks = copy.deepcopy(self.coin_stocks)

        for coin in Coin:
            while val - coin.value >= 0 and coin_stocks[coin.name] > 0:
                val -= coin.value
                coin_stocks[coin.name] -= 1
                my_coin_holder.coin_stocks[coin.name] += 1

        return my_coin_holder

    def has_enough_coin(self, change: int):
        val = change
        coin_stocks = copy.deepcopy(self.coin_stocks)
        for coin in Coin:
            while val - coin.value >= 0 and coin_stocks[coin.name] > 0:
                val -= coin.value
                coin_stocks[coin.name] -= 1

        if val == 0:
            return True

        return False

コイン関係の処理は全てCoinHolderが持つように責務を分離しているので、
自動販売機の方はCoinHolderのメソッドを使えば基本的に大丈夫になりました。

メソッドは限りなく小さくする

普段からメソッドは小さくするように心がけていましたが、
else句を使用しないというのは知りませんでした。
ただ以下の記事にあるように場合によってはelseも使った方が良いとも思うので、
早期リターンするべき状況をちゃんと使い分けられるように慣れていきたいです。
else句を使わないのが良いコードなの?いや、そんなはずは・・・

Beforeの合計メソッド数5個、平均行数10行
Afterの合計メソッド数20個、平均行数は5行
になりました。

今回の場合特に嬉しいのは自動販売機の持つbuyメソッドです。

Before

    def buy(self, i, kind_of_drink):

        if (i != 100) and (i != 500):
            self.change += i
            return None

        if (kind_of_drink == Drink.COKE) and (self.quantity_of_coke == 0):
            self.change += i
            return None
        elif (kind_of_drink == Drink.DIET_COKE) and \
                (self.quantity_of_diet_coke == 0):
            self.change += i
            return None
        elif (kind_of_drink == Drink.TEA) and (self.quantity_of_tea == 0):
            self.change += i
            return None

        if i == 500 and self.number_of_100_yen < 4:
            self.change += i
            return None

        if i == 100:
            self.number_of_100_yen += 1
        elif i == 500:
            self.change += (i - 100)
            self.number_of_100_yen -= (i - 100) / 100

        if kind_of_drink == Drink.COKE:
            self.quantity_of_coke -= 1
        elif kind_of_drink == Drink.DIET_COKE:
            self.quantity_of_diet_coke -= 1
        else:
            self.quantity_of_tea -= 1

        return Drink(kind_of_drink)

After

def buy(self, inserted_coins: CoinHolder, drink_type: Drink):
        self.vending_coin_holder.insert_coin(inserted_coins)
        if self._can_buy(drink_type) is False:
            return None
        self.vending_coin_holder.save_drink_coin(drink_type)
        return self.drink_holder.remove_drink(drink_type)

内部の処理は同じなのに
BeforeよりもAfterの方がbuyが何をするコードか理解しやすいのではないでしょうか?

まとめ

今回のような小さなプログラムだと
オブジェクト指向の良さがあまり分かりにくいかもしれませんが、
ドリンクの種類が増えたり、
水は80円だけどコーヒーは120円みたいにドリンクごとの値段が変わったり、
ランダムでもう一本あげるみたいな機能を追加しはじめると、
Afterのコードの方が変更が容易になるのではないかと思います。

反省

・どこまでクラス化するべきかの判断がまだ難しい。
例えばコインとかドリンクもenumではなく、クラスにした方が良いのかとか。

・いくつかのメソッドがNoneを返している。
自動販売機用の例外クラス作った方が良さそう。

改善した方が良いところだと指摘してもらえると幸いです。