鉄は熱いうちに打て
=============================

関数のreturnが変なコード

2017-12-22

12月は毎日書くぞー!と思って、10日分を残して早30日。いやー、12月は早かった!

ということで、残りの日付は今年の振り返りと小ネタで埋めていきます。

今日は私がコードレビューでよく指摘される話。

TogglとSlackを連携した話 のコードも最初、こうなっていました。

def get_toggl():
    〜略〜
    if current.status_code != 200:
        print("togglの情報取得に失敗しました")
    else:
        current_json = current.json()
        if current_json['data']:
            return current_json['data']['pid'], current_json['data']['description']
    return None, None

さあ、レビューしてみましょう!

  • 情報取得に失敗したあとも処理が続いている

  • 関数の最後で異常系(データがなかった場合)が返っている

  • 正常系がネストされたif文の中で返っている

名付けとか色々あると思いますが、私はよく言われダメコーディングの一つ、 関数のreturnを1回しか書かないやーつ なんです。気を抜くとすぐに書いてしまいます。 動けばいいやくらいで書いたスクリプトだったんで、ブログの記事を上げたら、旦那から「恥ずかしいから今すぐ直せ!」と言われました。(´・ω・`)。

def get_toggl():
    〜略〜
    if current.status_code != 200:
        print("togglの情報取得に失敗しました")
        return None, None

    current_json = current.json()
    if not current_json['data']:
        return None, None
    return current_json['data']['pid'], current_json['data']['description']

直したあとは↑みたいな感じ。関数の最後のreturnは正常系の最後にしたほうが読みやすいですね。

あと、私は 変なフラグをつくりたがるやーつ でもあります。気を抜くとすぐに flg_XXX みたいな変なフラグ変数を作っちゃいます。

まぁ、過去を振り返っても仕方ないので、これからも変なコードを作らないように自己レビューを厳しくしていきたいと思います。でも、変なときは指摘してくださいね。