関数内のfor文をリスト内包表記に書き替えたい
解決済
回答 1
投稿
- 評価
- クリップ 0
- VIEW 425
前提・実現したいこと
AWSのLambdaでpythonにて関数を作成したのですが、速度面を
若干改善したいと考えてfor文をリスト内包表記に書き替えたいのですが
どう記述していいか分かりません。
すいませんがご教示いただけますでしょうか。
該当のソースコード
get_table_valでDynamoDBからjson形式でデータを取得し、別のテーブルを
検索した値を追加してjsonで返すという様な処理をしています。
for以下をリスト内包表記に書き替えられないか考えておりますがうまくいきません。
def get_fee(id1,id2,key1,key2,tablename,age):
try:
t4 = os.getenv('TABLE_NAME4')
result=[]
item_content = get_table_val(id1,id2,key1,key2,tablename)
if(len(item_content) is not 0):
for rs in item_content:
rfee=get_table_val(id2+age,0,key1,0,t4)
if(len(rfee) is not 0):
rs['month_insurance_fee']=rfee[0]['month_insurance_fee']
rs['year_insurance_fee']=rfee[0]['year_insurance_fee']
else:
rs['month_insurance_fee']='－'
rs['year_insurance_fee']='－'
result.append(rs)
return result
except Exception as e:
LOGGER.error(e)
raise e試したこと
一応、for文を書き替えて、以下のコードでrsとrfee相当の値は取得できていますが
rfeeをrsに統合してリストに入れ込むところについてうまいアイデアが思いつきません。
rs = [ (i,get_table_val(id2+age,0,key1,0,t4)) for i in item_content]
なにかヒントになるようなことでも構わないのでご教示いただけますでしょうか。
補足情報（FW/ツールのバージョンなど）
AWS Lambda
python3.7
-
気になる質問をクリップする
クリップした質問は、後からいつでもマイページで確認できます。
またクリップした質問に回答があった際、通知やメールを受け取ることができます。
クリップを取り消します
-
良い質問の評価を上げる
以下のような質問は評価を上げましょう
- 質問内容が明確
- 自分も答えを知りたい
- 質問者以外のユーザにも役立つ
評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。
質問の評価を上げたことを取り消します
-
評価を下げられる数の上限に達しました
評価を下げることができません
- 1日5回まで評価を下げられます
- 1日に1ユーザに対して2回まで評価を下げられます
質問の評価を下げる
teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。
- プログラミングに関係のない質問
- やってほしいことだけを記載した丸投げの質問
- 問題・課題が含まれていない質問
- 意図的に内容が抹消された質問
- 過去に投稿した質問と同じ内容の質問
- 広告と受け取られるような投稿
評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。
質問の評価を下げたことを取り消します
この機能は開放されていません
評価を下げる条件を満たしてません
質問の評価を下げる機能の利用条件
この機能を利用するためには、以下の事項を行う必要があります。
- 質問回答など一定の行動
-
メールアドレスの認証
メールアドレスの認証
-
質問評価に関するヘルプページの閲覧
質問評価に関するヘルプページの閲覧
checkベストアンサー
+2
forループと内包表記は書き方の違いであってそれでパフォーマンスが向上するとかそういう類いのものではありません。
それはそれとして、
- rfeeは毎回同じ値が得られるはずなのでforの外側で取得すべき
- item_contentが空ならそもそもforループが回らないので、わざわざ空かどうか判定する必要がない
- result.append(rs) のインデントがおそらく間違い。forループの外に出てしまっている
- 0と比較するときに is や is not は使わない。単に ==
- 配列が空であることを判定するのに len は使わない。単に if rfee: とする
- rs の中身を書き換えるのは良くない。オブジェクトをイミュータブルに保つ
- tryのスコープを無意味に広げるべきでない
といったあたりを修正するとこうなります。
def get_fee(id1,id2,key1,key2,tablename,age):
try:
t4 = os.getenv('TABLE_NAME4')
item_content = get_table_val(id1,id2,key1,key2,tablename)
rfee=get_table_val(id2+age,0,key1,0,t4)
except Exception as e:
LOGGER.error(e)
raise e
if rfee:
month_insurance_fee = rfee[0]['month_insurance_fee']
year_insurance_fee = rfee[0]['year_insurance_fee']
else:
month_insurance_fee = '－'
year_insurance_fee = '－'
return [dict(rs,
month_insurance_fee=month_insurance_fee,
year_insurance_fee=year_insurance_fee,
) for rs in item_content]このほかには
- id1, id2, key1, key2 といった無意味な名前の変数名を付けるべきでない
- 関数内で環境変数を読み出すくらいなら、その値も引数として与える方が良い
- 関数が多くのことをしすぎているため、妥当な関数名を付けられず意味のないget_feeという名前になってしまっている
など修正すべき点はまだあります。
投稿
-
回答の評価を上げる
以下のような回答は評価を上げましょう
- 正しい回答
- わかりやすい回答
- ためになる回答
評価が高い回答ほどページの上位に表示されます。
-
回答の評価を下げる
下記のような回答は推奨されていません。
- 間違っている回答
- 質問の回答になっていない投稿
- スパムや攻撃的な表現を用いた投稿
評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。
15分調べてもわからないことは、teratailで質問しよう！
- ただいまの回答率 88.32%
- 質問をまとめることで、思考を整理して素早く解決
- テンプレート機能で、簡単に質問をまとめられる
2019/04/27 11:10
具体的なアドバイスまで頂戴できて非常に勉強になります。
・変数名については色々と勉強してますがなかなか追いつていません。色々見ながら精進していきたいと思います。
・環境変数の件、目から鱗でした。引数多くなるのはいけないのかなと思いましたがその方がシンプルになるんですね。
・妥当な関数名の件、役割分担とかそのあたりしっかりしないといけないです。
プログラムに苦手意識ありますが、きれいに書き直していただいたコードとか他の方の
コードとか見ながら頑張ってパフォーマンスが出せてかつ美しいコードを書けれるように
頑張っていきたいと思います。
2019/05/24 20:50
めちゃくちゃやばい実装ですね⋯