関数内のfor文をリスト内包表記に書き替えたい

前提・実現したいこと

AWSのLambdaでpythonにて関数を作成したのですが、速度面を
若干改善したいと考えてfor文をリスト内包表記に書き替えたいのですが
どう記述していいか分かりません。
すいませんがご教示いただけますでしょうか。

該当のソースコード

get_table_valでDynamoDBからjson形式でデータを取得し、別のテーブルを
検索した値を追加してjsonで返すという様な処理をしています。
for以下をリスト内包表記に書き替えられないか考えておりますがうまくいきません。

python
1def get_fee(id1,id2,key1,key2,tablename,age):
2  try:
3      t4 = os.getenv('TABLE_NAME4')
4      result=[]
5      item_content = get_table_val(id1,id2,key1,key2,tablename)
6      if(len(item_content) is not 0):
7         for rs in item_content:
8           rfee=get_table_val(id2+age,0,key1,0,t4)
9           if(len(rfee) is not 0):
10             rs['month_insurance_fee']=rfee[0]['month_insurance_fee']
11             rs['year_insurance_fee']=rfee[0]['year_insurance_fee']
12           else:
13             rs['month_insurance_fee']='－'
14             rs['year_insurance_fee']='－'
15         result.append(rs)
16      return result
17  except Exception as e:
18    LOGGER.error(e)
19    raise e

試したこと

一応、for文を書き替えて、以下のコードでrsとrfee相当の値は取得できていますが
rfeeをrsに統合してリストに入れ込むところについてうまいアイデアが思いつきません。

python
1   rs = [ (i,get_table_val(id2+age,0,key1,0,t4)) for i in item_content]

なにかヒントになるようなことでも構わないのでご教示いただけますでしょうか。

補足情報（FW/ツールのバージョンなど）

AWS Lambda
python3.7

行動規範の内容に同意します

回答1件

ベストアンサー

forループと内包表記は書き方の違いであってそれでパフォーマンスが向上するとかそういう類いのものではありません。

それはそれとして、

rfeeは毎回同じ値が得られるはずなのでforの外側で取得すべき
item_contentが空ならそもそもforループが回らないので、わざわざ空かどうか判定する必要がない
result.append(rs) のインデントがおそらく間違い。forループの外に出てしまっている
0と比較するときに is や is not は使わない。単に ==
配列が空であることを判定するのに len は使わない。単に if rfee: とする
rs の中身を書き換えるのは良くない。オブジェクトをイミュータブルに保つ
tryのスコープを無意味に広げるべきでない

といったあたりを修正するとこうなります。

python
1def get_fee(id1,id2,key1,key2,tablename,age):
2    try:
3        t4 = os.getenv('TABLE_NAME4')
4        item_content = get_table_val(id1,id2,key1,key2,tablename)
5        rfee=get_table_val(id2+age,0,key1,0,t4)
6    except Exception as e:
7        LOGGER.error(e)
8        raise e
9
10    if rfee:
11        month_insurance_fee = rfee[0]['month_insurance_fee']
12        year_insurance_fee = rfee[0]['year_insurance_fee']
13    else:
14        month_insurance_fee = '－'
15        year_insurance_fee = '－'
16
17    return [dict(rs,
18                 month_insurance_fee=month_insurance_fee,
19                 year_insurance_fee=year_insurance_fee,
20                 ) for rs in item_content]

このほかには

id1, id2, key1, key2 といった無意味な名前の変数名を付けるべきでない
関数内で環境変数を読み出すくらいなら、その値も引数として与える方が良い
関数が多くのことをしすぎているため、妥当な関数名を付けられず意味のないget_feeという名前になってしまっている

など修正すべき点はまだあります。

投稿2019/04/25 11:43

yuba

総合スコア5568

Otazoman

2019/04/27 02:10

ご回答ありがとうございます。具体的なアドバイスまで頂戴できて非常に勉強になります。・変数名については色々と勉強してますがなかなか追いつていません。色々見ながら精進していきたいと思います。・環境変数の件、目から鱗でした。引数多くなるのはいけないのかなと思いましたがその方がシンプルになるんですね。・妥当な関数名の件、役割分担とかそのあたりしっかりしないといけないです。プログラムに苦手意識ありますが、きれいに書き直していただいたコードとか他の方のコードとか見ながら頑張ってパフォーマンスが出せてかつ美しいコードを書けれるように頑張っていきたいと思います。