間違えたif文とフラグの使いかた

Question

お世話になります。
５つの分岐をして、結果的に条件に合わない場合は、成功と考えて

arbitrage_decision = 1 # 停止がない為、裁定取引判定処理を実施（一番下の行です）

を実行したいのですが、あまりに長くカッコ悪いので
別のみやすい書き方はないでしょうか？難しいすぎるようなコードは読めませんので適度に見やすい方が良いですが
これは、なんかとても長いイメージがあります。

    if net_profit_price < 0:
          stop_process_no1 = stop_process_no1 + 1
          arbitrage_decision = 0  
    if net_ask_price > risk_limit_price:  #
          stop_process_no2 = stop_process_no2 + 1
          arbitrage_decision = 0  
    if net_bid_price > risk_limit_price:  #
          stop_process_no3 = stop_process_no3 + 1
          arbitrage_decision = 0  
    if net_profit_price < minimum_profit_price:
          stop_process_no4 = stop_process_no4 + 1
          arbitrage_decision = 0  
    if net_profit_rate < minimum_profit_rate:
          stop_process_no5 = stop_process_no5 + 1
          arbitrage_decision = 0  
    if (net_profit_price < 0):
        pass
    else:
        if (net_ask_price > risk_limit_price):
            pass
        else:
            if (net_bid_price > risk_limit_price):
                pass
            else:
                if (net_profit_price < minimum_profit_price):
                    pass
                else:
                    if (net_profit_rate < minimum_profit_rate):
                        pass
                    else:
                        arbitrage_decision = 1

Answer 1

bool_list = [net_profit_price < 0,
             net_bid_price > risk_limit_price,
             net_ask_price > risk_limit_price,
             net_profit_price < minimum_profit_price,
             net_profit_rate < minimum_profit_rate]

if not any(bool_list):
    arbitrage_decision = 1

anyを使えばリスト内のいずれかがtrueであればtrue
全てがfalseであればfalseを返してくれます。

not any とする事で全てがfalseならばtrueとする事が出来ます。

Comments

[退会済みユーザー]

素敵ですね。ただそれぞれの処理のカウントアップをしたいのですが、そういった時はどう書けばいいですか？stop_process_no1 = stop_process_no1 + 1

[puroko3]

すいませんこれはelse後の無駄なpassを無くすためのコードです。

[hayataka2049]

not anyにしないと駄目なのでは

[puroko3]

確かにそうですね...失礼しました。修正させていただきます。

[can110]

全てがfalseならばtrueとするのであれば not all ではなく not anyでは？
print( not all([False,True])) # True????
print( not any([False,True])) # False
print( not all([False,False]))# True
あっと失礼。指摘かぶりました。
print( not any([False,False]))# True

[hayataka2049]

修正後の「全てがfalseならばfalseとする事が出来ます」の説明が間違っているのではないでしょうか

[puroko3]

何度もすいません... 修正させていただきました

[hayataka2049]

「いずれか1つ以上がtrueならfalse」、「すべてがfalseならtrue」なのでまだ間違ってますね・・・

[puroko3]

すいません完全にこんがらがってました。今度こそ大丈夫だと思います。

[hayataka2049]

一度に書かなかった私が悪いのですが、上の「anyを使えばリスト内のいずれかがtrueであればfalseを返してくれます。」の方も・・・

[puroko3]

もっと落ち着かないといけないですねえ... 今度こそ大丈夫かと思います...

[退会済みユーザー]

ありがとうございます。えっとこれでOKでしょうか？かっこいいコードですね！

[退会済みユーザー]

みんなの指摘が交差しててかっこいいなー

Answer 2

複雑な条件のIF文を一つにまとめると、
デバック時に追いにくいので私なら以下のようにします。

check1=net_profit_price < 0
check2=net_ask_price > risk_limit_price 
check3=net_bid_price > risk_limit_price
check4=net_profit_price < minimum_profit_price
check5=net_profit_rate < minimum_profit_rate

    if check1:
          stop_process_no1 = stop_process_no1 + 1
          arbitrage_decision = 0  
    if check2:  #
          stop_process_no2 = stop_process_no2 + 1
          arbitrage_decision = 0  
    if check3:  #
          stop_process_no3 = stop_process_no3 + 1
          arbitrage_decision = 0  
    if check4:
          stop_process_no4 = stop_process_no4 + 1
          arbitrage_decision = 0  
    if check5:
          stop_process_no5 = stop_process_no5 + 1
          arbitrage_decision = 0

if(!check1 and !check2 and !check3 and !check4 and !check5)
{
  arbitrage_decision = 1
}

Answer 3

条件が５つなら、それぞれ番号つけて、後でまとめて処理するほうがわかりやすいとおもいますぜ
・・・って、switch あればなあ

  hoge=0
  if(aaa):
    hoge=1;
  if(bbb):
    hoge=2;
  if(ccc):
    hoge=3;
  if(ddd):
    hoge=4;
  if(eee):
    hoge=5;

  if(hoge<=0):
    その他の処理
  if(hoge==1):
    aaa の処理
  以下略

Comments

[退会済みユーザー]

素敵ですね。ただそれぞれの処理のカウントアップをしたいのですが、そういった時はどう書けばいいのでしょうか？stop_process_no1 = stop_process_no1 + 1

[y_waiwai]

他の回答にありますが、カウントアップするなら
stop_process_no1 += 1
って記述でOKですねー

[退会済みユーザー]

ありがとうございます！

Answer 4

条件を逆にしてandで繋げば一つのifで済むのでは？

if (net_profit_price >= 0 and 
    net_ask_price <= risk_limit_price and
    net_bid_price <= risk_limit_price and
    net_profit_price >= minimum_profit_price and
    net_profit_rate >= minimum_profit_rate):
    arbitrage_decision = 1

追記

stop_process_dict = {0:0, 1:0 2:0, 3:0, 4:0}
state_lst = [net_profit_price < 0, 
             net_ask_price > risk_limit_price,
             net_bid_price > risk_limit_price, 
             net_profit_price < minimum_profit_price,
             net_profit_rate < minimum_profit_rate]

for i, state in enumerate(state_lst):
    if state:
        stop_process_dict[i] += 1
        arbitrage_decision = 0  

if not any(state_lst):
    arbitrage_decision = 1

変数を変えてしまうのが前提のコードです。なので他の部分にも修正が及びます（stop_process_no1はstop_process_dict[0]等とします）。

Comments

[退会済みユーザー]

ありがとうございます！こういうのがやりたかったです。今度からはこのようなコードをかけるようになりたいです。本当にありがとうございました。

[退会済みユーザー]

それぞれの処理のカウントアップをしたいのですが、そういった時はどう書けばいいのでしょうか？stop_process_no1 = stop_process_no1 + 1

[hayataka2049]

上半分の部分にはいろいろな問題があって、あまり手をつけたくないのですが
・そもそも似たような変数が5つもあるとわかりづらい
・下と同じ判定なのでコードがかぶってわかりづらい（そしてバグの温床になる）
そうすると、他の方の回答に上がっているように、booleanの配列を先に作るのがよさそうです。stop_process_no*はすべてコレクション型で管理するようにし、
・booleanの配列をループで回す→コレクション型の該当する要素をインクリメントする
・all(booleanの配列)ですべてのandになるので、それでifの判定をしてarbitrage_decision=1の処理をする
こんな方針でどうでしょうか

[退会済みユーザー]

しかしカウントアップはそれぞれに必要ですよね。
stop_process_no2 += 1

[hayataka2049]

記述をまとめてしまうことはできるでしょう。ちょっと追記してみます

[退会済みユーザー]

ありがとうございます。宜しくお願いいたします。とても素敵なコード個人的には一番好きだったのでぜひよろしくお願いします。

[hayataka2049]

結局not anyにしました（逆転させておいていちいちnot入れるのもバカバカしかった）。コメントにも書いた通り変数をぜんぶstop_process_dictに押し込んでいるので、今までどおりstop_process_no1等と簡単にアクセスすることはできなくなりましたねー。でもこの方が余計な苦労がないと思います

[hayataka2049]

キーを0,1,2,3,4にするならリストでも良いか・・・付けたい名前があれば付けてあげてください。ただしforループでアクセスするとき、そのキーのリストとstate_lstをzipで回す等の変更が必要になります

[退会済みユーザー]

ありがとうございます。なるほどー。ちょと私には難しいです。

[hayataka2049]

そのまま使う必要はないので、いろいろな人のコードを見て勉強の参考にすれば良いです

[退会済みユーザー]

いつもありがとうございます！引き続きよろしくお願いします。

Answer 5

変数保持の仕方に根本的な問題があると思いますが…

    import operator
    def process(val, thr, pro, dec, comp=operator.lt):
        if comp(val, thr):
            return pro+1, 0
        else:
            return pro, dec
    
    stop_process_no1, arbitrage_decision = process(net_profit_price,
                                                   0,
                                                   stop_process_no1, arbitrage_decision)
    stop_process_no2, arbitrage_decision = process(net_ask_price,
                                                   risk_limit_price,
                                                   stop_process_no2, arbitrage_decision,
                                                   comp=operator.gt)
    stop_process_no3, arbitrage_decision = process(net_bid_price,
                                                   risk_limit_price,
                                                   stop_process_no3, arbitrage_decision,
                                                   comp=operator.gt)
    stop_process_no4, arbitrage_decision = process(net_profit_price,
                                                   minimum_profit_price,
                                                   stop_process_no4, arbitrage_decision)
    stop_process_no5, arbitrage_decision = process(net_profit_rate,
                                                   minimum_profit_rate,
                                                   stop_process_no5, arbitrage_decision)
    
    if (net_profit_price >= minimum_profit_price and
        net_ask_price <= risk_limit_price and
        net_bid_price <= risk_limit_price and
        net_profit_rate >= minimum_profit_rate):
        arbitrage_decision = 1

Answer 6

おそらく題意に沿わないので回答ではありませんが
各判定をリストで管理し、かつその結果を積算するコードを考えてみました。

err_cnt = [0,0,0,0,0] # 各判定結果（エラー数）
for (a,b) in [(1,2),(11,101),(101,1001)]: # テスト

    err_list = [a > 10, a > 100, b > 10, b > 100, b > 1000] # True=エラー
    print(a,b)
    print(err_list)

    # エラー数の積算
    err_cnt = [ c + l for (c,l) in zip(err_cnt, err_list)]
    print(err_cnt)

    if not any(err_list): # いずれかTrue(エラー) の逆 = すべてFalse(正常)
        print('pass!')
    else:
        print('ng!')

Answer 7

質問からは少し外れますが、 stop_process_no1 = stop_process_no1 + 1 は、stop_process_no1 += 1 とした方がいいですね。

Comments

[退会済みユーザー]

ありがとうございます。書き直します。

Answer 8

if (net_profit_price < 0): #判定No1
        return 0

if (net_ask_price > risk_limit_price): # 判定No2
        return 0

if (net_bid_price > risk_limit_price): # 判定No3
        return 0

if (net_profit_price < minimum_profit_price): # 判定No4
        return 0

if (net_profit_rate < minimum_profit_rate): # 判定No5
        return 0

return 1  # 停止がない為、裁定取引判定処理を実施

質問文のコードを1つの関数にし、上記のようにしてはどうでしょうか？
（メッセージ出力や各種変数への値の代入は端折りました。）
関数の呼び出し元からは、返り値を見て裁定取引判定処理を実施するかどうかを判断できます。

Comments

[退会済みユーザー]

丁寧にありがとうございました。
if (net_profit_price >= 0 and
net_ask_price <= risk_limit_price and
net_bid_price <= risk_limit_price and
net_profit_price >= minimum_profit_price and
net_profit_rate >= minimum_profit_rate):
arbitrage_decision = 1
こっちの方が良いのでこっちにします。これからもよろしくお願いします。

[退会済みユーザー]

それぞれの処理のカウントアップをしたいのですが、そういった時はどう書けばいいのでしょうか？stop_process_no1 = stop_process_no1 + 1