自販機のプログラムコードを添削してほしい

Question

簡単な自販機のプログラミングを書いたのですが、周囲の添削してくれる人がいないので添削して頂きたいです。

主に添削してほしい箇所はコメントで記載しています。
しかし、全体としてコードが長く見ずらい部分が多々あるので、どこを関数化するなどのアドバイスも頂けますと幸いです。

前提条件として

・お札投入口はない
・投入は複数回
・投入金額の総計を取得できる
・払い戻しで、投入金額の総計をつり銭として出力

の4つがあります。

どなたか、ご指導ご鞭撻のほどよろしくお願いいたします。

drink_lists = [["Cola",120,5],["Soda",100,4]]
sum = int(0)
sales_profit = int(0)

while True:
    select_button = input("chose button,1:投入,2:払い戻し,3:購入,4:在庫確認 \n")
    
    if int(select_button) == 1:
        
        print("使用できる通貨は,10円,50円,100円,500円の4種類です。　紙幣・1円と5円は使用することができません")
        paid_in = input("金額を投入してください")
        paid_in = int(paid_in)
        
        #もっと簡略化できないか？
        if paid_in == 10 or paid_in == 50 or paid_in == 100 or paid_in == 500:
            sum = sum + paid_in
            print("金額合計:",sum)
        
        else:
            print("指定外の硬貨が投入されました")
            break
        
    elif int(select_button) == 2:
        print("払い戻しが選択されました \n 払い戻し金額:",sum)
        sum = 0
        break
    
    elif int(select_button) == 3:
        print("購入が選択されました \n 現在の投入金額:",sum)
        
        i = int(0)
        
        for drink_list in drink_lists:
            print(i,":",drink_lists[i][0]) #print(drink_list[i][0])ではエラーが出る
            i += 1
            
        select_drink = input("購入する商品を選んでください \n")
        
        #select_drink が数字の場合
        if select_drink.isdigit():
            select_drink = int(select_drink)
            #商品増加の都度、足していく必要がある？
            if sum >= drink_lists[select_drink][1] and drink_lists[select_drink][2] >= 0:
                print(drink_lists[select_drink][0] + "を購入しました\n")
                #金額計算
                sum = sum - drink_lists[select_drink][1]
                #在庫減少
                drink_lists[select_drink][2] -= 1
                #売上計算
                sales_profit = sales_profit + drink_lists[select_drink][1]
            
            elif sum < drink_lists[select_drink][1]:
                print("投入金額が足りません")
            
            elif drink_lists[select_drink][2] == 0:
                print("在庫がありません")
            
            """
            存在しない商品を選ぶことはない前提で作る
            else:
                print("商品がありません")
                break
                
            """
        else:
            print("商品を選択してください")
    
    elif int(select_button) == 4:
        print("売上は:",sales_profit,"円です")
        i = 0
        for drink_list in drink_lists:
            #print(drink_list[i][0])ではエラーが出る
            print("名前:",drink_lists[i][0],"値段",drink_lists[i][1],"在庫",drink_lists[i][2])
            i += 1
            
    # 操作選択において、1,2,3,4以外(文字)が入力されたときの処理
    else:
        print("始めからやり直してください")
                

Comments

[y_waiwai]

実際に動かして、きちんと動いてるんでしょうか

[yudouhu]

大丈夫です。
動作確認はしており、きっちりと動いております。

Answer 1

classの勉強はまだですかね？
添削ではなく、classを使った書き方を示しておきます。
役割毎にクラスを分けます。main関数: ユーザとのやりとり担当、VendingMachineクラス: 自動販売機
自動販売機の中に、商品を入れるレーン（Stocker:在庫管理担当）を置くのが楽かと思います。

class Stocker:

    def __init__(self, name, price, stock):
        self._name = name
        self._price = price
        self._stock = stock

    @property
    def name(self):
        return self._name

    @property
    def price(self):
        return self._price

    @property
    def stock(self):
        return self._stock

    def eject(self):
        if self.stock <= 0:
            raise ValueError("在庫がありません")
        self._stock -= 1
        return self.name


class VendingMachine:

    def __init__(self, stockers):
        self._stockers = dict(enumerate(stockers, 1))
        self._money = 0
        self._sales = 0

    @property
    def money(self):
        return self._money

    @property
    def sales(self):
        return self._sales

    def lineup(self):
        for number, stocker in self._stockers.items():
            yield number, stocker.name, stocker.price

    def status(self):
        for stocker in self._stockers.values():
            yield stocker.name, stocker.price, stocker.stock

    def insert(self, money):
        if money not in (10, 50, 100, 500):
            raise ValueError("指定外の金額が投入されました")
        self._money += money

    def cancel(self):
        money = self._money
        self._money = 0
        return money

    def buy(self, number):
        if number not in self._stockers:
            raise ValueError("商品を選択してください")
        price = self._stockers[number].price
        if self.money < price:
            raise ValueError("投入金額が足りません")
        self._money -= price
        self._sales += price
        return self._stockers[number].eject()


vending_machine = VendingMachine([
    Stocker(name="Cola", price=120, stock=5),
    Stocker(name="Soda", price=100, stock=4),
])


def insert():
    print("使用できる硬貨は,10円,50円,100円,500円の4種類です。")
    print("紙幣と1円と5円は使用することができません。")
    money = input("硬貨を投入してください: ")
    if not money.isdigit():
        print("硬貨ではありません")
        return
    try:
        vending_machine.insert(int(money))
        print(f"投入合計: {vending_machine.money}円")
    except ValueError as error:
        print(str(error))


def cancel():
    print("払い戻しが選択されました")
    money = vending_machine.cancel()
    print(f"払い戻し金額: {money}円")


def buy():
    print("購入が選択されました")
    print("現在の投入金額:", vending_machine.money)
    for number, name, price in vending_machine.lineup():
        print(f"{number}: {name} {price}円")

    selection = input("購入する商品の番号を選んでください: ")
    if not selection.isdigit():
        print("商品がありません")
        return

    try:
        drink = vending_machine.buy(int(selection))
        print(f"{drink}を購入しました")
    except ValueError as error:
        print(str(error))


def status():
    print(f"売上は {vending_machine.sales}円です")
    for name, price, stock in vending_machine.status():
        print(f"商品名:{name}、価格:{price}、在庫:{stock}")


def main():
    actions = {
        "1": insert,
        "2": cancel,
        "3": buy,
        "4": status,
    }
    while True:
        print()
        selection = input("選択 1:投入、2:払い戻し、3:購入、4:在庫確認 ? ")
        if selection in actions:
            actions[selection]()
        else:
            print("始めからやり直してください")


if __name__ == '__main__':
    main()

Comments

[yudouhu]

shiracamusさん、ありがとうございます。
クラスの勉強はまだというよりも、一度コードに落としてからでないと考えられないという具合です。

また、クラスを使った書き方は大変見やすいコードになっているので、参考にさせて頂きます。
重ね重ね、ありがとうございました。

Answer 2

どういう観点で見てほしいのかよくわからないので、思ったことをいくつか書いてみます。

• 特に長いとは思わないし、それなりにきれいに書けていると思います。

• 関数化するのは、select_buttonの各処理の内容ごとがいいと思います。
  関数内で処理をして処理結果を返してループを制御する感じにします。

• 異常な入力の時の処理に一貫性が無い
  購入時に数値以外を入れると、商品を選択しろと出るのに処理選択に戻ってしまうとか、想定外の金額を入れると終了してしまうとか。

• プログラミング的なアドバイス

i を使って数えているところは enumerateを使うとすっきりします。

     i = int(0)
        
    for drink_list in drink_lists:
        print(i,":",drink_lists[i][0]) #print(drink_list[i][0])ではエラーが出る
        i += 1

        for i, drink_list in enumarate(drink_lists):
            print(i,":",drink_lists[i][0]) #print(drink_list[i][0])ではエラーが出る

#もっと簡略化できないか？

上のものは下のように書けます。

        if paid_in == 10 or paid_in == 50 or paid_in == 100 or paid_in == 500:

        if paid_in in [10, 50, 100, 500]:

print(drink_list[i][0])ではエラーが出る

drink_lists[i][0] でしょう。 s が抜けています。

---

こんなところです。

Answer 3

いろいろ仕様が変な気がしますが（値段不明の商品を購入する、在庫の無い商品まで表示する等）、そこはさておくと、
・無意味な空行がある
・sum = int(0)などで、何故int関数を使っているのか意味不明
・for文の理解が足りない
・入力を、文字列として扱うか数値に変換して扱うかの考慮が足りてない
・変数名が変（profitは「利益」ですが?等）
あたりが、気になるところです。

仕様はそのままで書き直すと、

drink_list = [["Cola",120,5],["Soda",100,4]]
paid = 0
sales = 0

while True:
    select_button = input("chose button,1:投入,2:払い戻し,3:購入,4:在庫確認 \n")

    if select_button == "1":
        print("使用できる通貨は,10円,50円,100円,500円の4種類です。　紙幣・1円と5円は使用することができません")
        coin = input("金額を投入してください")
        if coin in ["10","50","100","500"]:
            paid += int(coin)
            print("金額合計:",paid)
        else:
            print("指定外の硬貨が投入されました")

    elif select_button == "2":
        print("払い戻しが選択されました \n 払い戻し金額:",paid)
        paid = 0

    elif select_button == "3":
        print("購入が選択されました \n 現在の投入金額:",paid)
        for i, drink in enumerate(drink_list):
            print(i,":",drink[0])

        select_drink = input("購入する商品を選んでください \n")
        if select_drink.isdigit():
            drink = drink_list[int(select_drink)]
            if paid < drink[1]:
                print("投入金額が足りません")
            elif drink[2] == 0:
                print("在庫がありません")
            else:
                print(drink[0] + "を購入しました\n")
                paid -= drink[1]
                drink[2] -= 1
                sales += drink[1]
            """
            存在しない商品を選ぶことはない前提で作る
            else:
                print("商品がありません")
                break

            """
        else:
            print("商品を選択してください")

    elif select_button == "4":
        print("売上は:",sales,"円です")
        for drink in drink_list:
            print("名前:",drink[0],"値段",drink[1],"在庫",drink[2])

    # 操作選択において、1,2,3,4以外(文字)が入力されたときの処理
    else:
        print("始めからやり直してください")

仕様そのままと書きましたが、
・数字じゃないものを入力するとエラー終了するのを改善
・在庫ゼロでも買えてしまうのを改善
・リストに無いコイン入力時や払い戻し時にプログラム終了するのも何だかなと思うので、breakを削除
だけ変えています。

Comments

[yudouhu]

otnさん、ご教示ありがとうございます。
for文とenumerate関数をもう少し勉強します。

TakaiYさんもご指摘ありがとうございました。
関数のアドバイスは参考にさせて頂きます。

[otn]

全部自力で書いたのであれば、改善点はあった物の、在庫ゼロの場合以外はバグも無く、よく書けていると思います。

関数化するのであれば、一旦関数無しで書いてあとから関数にして外に出すというより、
設計の最初から、機能分割を考えて処理を書くのでしょうね。

[yudouhu]

＞関数化するのであれば、一旦関数無しで書いてあとから関数にして外に出すというより、
設計の最初から、機能分割を考えて処理を書くのでしょうね。

otnさん、アドバイスありがとうございます。
まだまだその域には達していませんが、コードを書く際の参考にさせて頂きます。

Answer 4

レビュー以前の問題が２つ。

1. lint チェックする
2. テストを書く

lint 実行例