3
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 1 year has passed since last update.

ハンズラボAdvent Calendar 2022

Day 18

リーダブルコードを読んで過去書いたコードを振り返る(表面上の改善編)

Last updated at Posted at 2022-12-17

本記事は、ハンズラボ Advent Calendar 2022 18日目の記事です。

はじめに

断捨離していたら新卒の時に購入したリーダブルコードを見つけたので読んでみました。
本記事では、リーダブルコード第1部「表面上の改善」の内容を自分の解釈を踏まえて、過去に書いたコードや読んだコードを振り返ります。

本題

理解しやすいコード

以下は、三項演算子を入れ子で使っているパターンです。
ワンライナーで書かれていてとてもトリッキーなことをしているように見えます。
このコードを最初見た時、意味がわかりませんでした。

this.isDirty ? this.validFunc ? this.validFunc(this.modelValue) : this.errMessage ? false : true : null;

三項演算子はかっこいいし便利ですが以下の書き方の方が読みやすいと思います。
行数は多いですが内容は同じです。

if (this.isDirty) {
	if (this,validFunc) {
		this.validFunc(this.modelValue);
	} else {
		this.errMessage ? false : true;
} else {
	null;
}

コードは短くした方がいいが「理解するまでにかかる時間」を短くてするほうが大切だということです。

  • コードは他人が最短時間で理解できるように書かなければいけない。
  • 常に俯瞰して「このコードは理解しやすいだろうか?」と自問自答すると良い。

汎用的な名前は避ける

以下コードは汎用的な名前の変数tmpを使ったものです。
複数のエラーメッセージを整形して表示するために、要素のおしりに\nをつけて「一時的な保管場所」的な意味でtmp変数に追加しています。
当時はこの変数はいい名前思い浮かばないし生存期間は短いためtmpでいいだろうと考えていたと思います。
この考えは怠慢です。

err_msg_list = [...]

tmp = ''
for err_msg in err_msg_list:
    tmp += err_msg + '\n'

title = 'エラー'
message = f'以下内容のエラーが発生しています。  ` ` ` {tmp} ` ` ` '
    
# Slack 投稿
post_slack(title, message)

tmpからは何も情報を得ることができないため、他人がコードを見た時の理解やバグの発見などに役立ちません。
変数名や関数名には汎用的な名前をつけるのではなく、対象物の値や目的を現した名前を選ぶべきです。
汎用的な名前(tmp、it、retvalなど)でも問題ない場合もありますが、避けるべきだということです。
今変数名を付け直すとしたらdisplay_err_msgとかかな...?

コメントはコードの意図を書く

以下は受け取ったイテレータブルオブジェクトの最後の一つの要素の時にTrue、それ以外の時にFalseを返す関数です。
このコードはプログラムの動作をそのまま書いているだけで、この関数は何をするためのものなのかわかりません。

def last_one(iterable):
    # イテレータを取得して最初の値を取得する
    it = iter(iterable)
    last = next(it)
    # 2番目の値から開始して反復子を使い果たすまで実行
    for val in it:
        # 一つ前の値を返す
        yield last, False
    # 最後の一つ
    yield last, True

一行ずつプログラムの動作を説明するのではなく、関数の機能を説明します。
イテレータの要素を頭から見ていって最後の要素の時に+αの処理をしたいときに使用する関数であることをコメントで残したいです。
コメントはプログラムの動作の詳細を書くより、プログラムの動作を高レベルから説明すると、他人から見た時の理解やバグ発見に役立ちます。
例が悪いのでコードの続きを書きます。

...

# 指定された番号のオーダーを削除する
sql = '''
    DELETE FROM
        order
    WHERE
        no IN (
'''

list = [1, 2, 3]
args = ()
# SQL文のIN句を組み立てるために最後の要素は括弧閉じ
for item, is_last in last_one(list):
    if is_last:
        sql += "%s)"
    else:
        sql += "%s, "
    args = args + (item,)

dao.delete(sql, args)

おわりに

自分が書いてきたコードを一部振り返ってみて、表面的なことだけでもまだまだ改善できると思いました。
今後は他人が理解しやすいコードを意識してコーディングしていこうと思います。
あと第2部以降もそのうち書きたいと思ってます。

3
0
2

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
3
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?