qqemm9
@qqemm9

Are you sure you want to delete the question?

Leaving a resolved question undeleted may help others!

ソースコードの改善について

Q&A

Closed

解決したいこと

問題の解決ではなく、掲載するソースコードの改善点の指摘をお願いします。
このプログラムの目的は、代金と所持金を入力すると、財布の中の小銭の枚数が最小になるよう計算するものです。
フィールド名、メソッド名に関する指摘や、ここはこの処理の方が効率がいいなどといったことを教えて頂きたいです。
よろしくお願いします。

該当するソースコード

# お金の種類
money_type = [1000, 500, 100, 50, 10, 5, 1]
# お金の種類の数、1000円~1円までの7種類
number = len(money_type)


def getQuotientAndReminder(money, value):
    """ 金額の除算の商(整数)とあまりを返す """
    # 商
    quotient = money / value
    # あまり
    reminder = money % value
    return int(quotient), reminder


def setPurse(money):
    """ 金額における紙幣・小銭の数を取得してリスト形式にして返す """
    reminder = money
    purse = []
    for i in range(number):
        quotient, reminder = getQuotientAndReminder(reminder, money_type[i])
        purse.append(quotient)
    return purse


def subtractPurse(X, Y):
    """ 小銭の枚数の差を計算 """
    Z = []
    for (x, y) in zip(X, Y):
        Z.append(x - y)
    return Z


def getAllMoney(coins):
    """ 小銭の枚数から金額の合計を計算 """
    money = 0
    for (c, mt) in zip(coins, money_type):
        money += c * mt
    return money


def calculate(price, in_hand_money):
    """ 小銭の最小化を計算 """

    print("代金:" + str(price) + ", 所持金:" + str(in_hand_money))

    # http://takeno.iee.niit.ac.jp/~shige/math/lecture/misc/data/kozeni2.pdf
    # 上記リンク、「4 実際の支払時の応用」のA,B,C,A'(,B')を使用する

    # A 所持金の小銭
    A = setPurse(in_hand_money)
    # B 最終的な残金
    B = setPurse(in_hand_money - price)
    # C AとBの共通部分
    C = []
    for (a, b) in zip(A, B):
        if a == 0 or b == 0:
            C.append(0)
        elif a > b:
            C.append(b)
        elif b > a:
            C.append(a)
        elif a == b:
            C.append(a)

    # A' AからCを取り除いたもの
    A_dash = subtractPurse(A, C)
    # B' BからCを取り除いたもの
    B_dash = subtractPurse(B, C)

    # 店員に渡す金額
    print("店員に渡す金額:" + str(getAllMoney(A_dash)))
    # おつり
    print("お釣り:" + str(getAllMoney(B_dash)))
    # 清算後の所持金
    print("清算後の所持金:" + str(in_hand_money - getAllMoney(A_dash) + getAllMoney(B_dash)))


if __name__ == '__main__':
    # 入力: calculate(代金, 所持金)
    calculate(716, 1324)
    calculate(694, 1448)

0

2Answer

メソッド名は get_quotient_and_reminder のようにスネークケースにするのが Python の流儀です。(他の言語から移植されたライブラリは流儀から外れることもあります。)ローカル変数名も一般的には小文字だけ使いますが、与えられた数式があるなら表記を合わせたほうが無難なので A_dash などでいいと思います。グローバルかつ変更しない変数は MONEY_TYPES のようにします。リストなので複数形ですね。

商の整数値は money // value で得られます。

for i in range(number): はリストを直接イテレートする for type_ in money_type: のほうがシンプルです。

setPurse 関数の名前に違和感があります。 set は引数で受け取ったオブジェクトに別の何かをセットする(メソッドならレシーバに何かをセットする)ときに使うのが普通です。この場合は get_pursecalculate_purse がいいと思います。

文字列の組み立ては Python 3.6 以降では f-string を使って print(f'代金:{price}, 所持金:{in_hand_money}') のように書けます。

非効率な箇所は特に見当たりませんでした。

3Like

Comments

  1. @qqemm9

    Questioner

    ありがとうございます!
    大変参考になります!

Comments

  1. @qqemm9

    Questioner

    ご指摘ありがとうございます!

Your answer might help someone💌