何か着ていればいいよ

ソフトウェア技術者の日常や技術の話を書こうと思います。

コードレビュー中に『う゛んぎぃ!!!』ってなった話

とあるシステムのコードレビュー中の話

こんなコードを見てイライラした。
※変数名やコメントは業務コードだったので実際のものとは変えています。

この短いセンテンスの中にイラッと来るポイントが複数あって凄い!

  • if文ごとにコメントがあるがifの条件をまんま書いてるだけで意味ない
    • ていうか全般的にコメントはコードそのままの内容でほとんど意味ない
  • 3,4行目
    • コメントとif文の処理内容が異なる(この時点でバグか!?と思わせる)
    • コメントとif文の大小比較演算子の向きが逆。(なぜわかりづらくするのか?)
  • 9,10行目
    • なんでelse ifじゃないんだ!となる。そして何かに気づく…。
      • 90の場合、こちらのif文にもはいってくるので結果としてfugaの値は正しい結果になっているっぽい。

じゃあどう直すべき?

結果が負になる場合とか、intとdoubleのキャストなんかの考慮も必要なんだろうけどはしょって簡単に書くと

fuga = 10 - intValu / 10

という感じで良いんじゃないかな?

結局何が言いたいか?

この元コードからは様々なアンチパターンというか良くないところが読み取れるけれど…。
論理を積み上げていけば、そういうまずい部分は一つづつ解消出来るのではないだろうか?
*1

*1:というふうにきれいにまとめたい。でも、本当はこんなん見るはめになってプリプリしたので吐き出しておきたかったという所