リファクタリングが自分を成長させてくれる
前回の記事では、どんな初心者でも「投票システム(フロント)」を実装できるっていうことを紹介したんだけど、当然ググってコピペ、そのコードをひたすらコピペで量産では、ごみコード(またの名をウンコード)になるのはいうまでもない。なので前回の状態からレビューをしてくれる架空の先輩エンジニ …
リーダブルコードを心がけよう。ってよく言われてきた。このサイトでは処理の書き方とかにそこまで拘らずとにかくプログラミングすることが重要だってさんざん書いてきた。初心者のうちはラムダ式とかinterfaceとかそういう知識より、とにかく何かを作り上げることが重要だということ。一方で、読みやすいコードを書くっていうのはまた別の話だ。いかにリーダブルかってのは、処理の書き方云々の話ではなく単純な読みやすさの話。今回はこのリーダブルコードのほんの一部について伝授したいと思う。そしてこの能力もあなたのプログラミング人生を大きく変えるものとなる。
ごりごりとコードを書いていくとき、後述のようなダーティな書き方をしている場合、他人がそれを見ても理解できないかもしれない。正確にいうと、理解できないのではなく理解するのにコストがかかるので読みたくなくなるってこと。また誤解を与えてしまうこともあるかもしれない。そうなると保守性も低くなるわけだ。
「他人にコードを見せることはないやい!俺だけがいじっていくコードなんだい!」っていう孤独なあなた。未来の自分は本当にあなたの汚いコードを難なく理解してくれるだろうか?正直6か月も前に書いたコードのことなんて覚えてられない。つまり、将来の自分自身のためにもリーダブルコードを意識することが重要なのだ。
プログラミングをする上で、英語能力は高いほうが間違いなく良い。多くの有用な情報は英語で書かれていることが多いし。特にstackoverflowにはみんなが世話になるよね。またコードを書く時にも後述するが、英語の語学力が高い方が、誰にとっても読みやいすいコードを書くことができる。まぁ日本語でプログラムすることも可能だけど、エディタのサジェスト機能などを考えると英語を使用するほうがいいよね。
もしあなたが英語が苦手だとしても心配することはない。英語のサイトを読むときは自動翻訳をかけて読めばいいじゃん。また、プログラミングで出てくる英単語、フレーズはほんの少しなので、やっていくうちにすぐプログラミングに関する英語語彙力が向上していく。とにかく英語に苦手意識を持たないようにしなければいけない。
大げさに以下の汚いコードを用意してみたが、まれに実世界でこのようなコードを書く人に出会うことがある。ちなみに今回はC#でのコード例だが、C#のことを知らない人でもリーダブルが何たるかを理解できるからご心配なく。
public bool CheckUser(int i)
{
// データベースからidが一致するユーザー情報を取得する
var data = _context.User.where(u => u.Id == i).First();
if (data != null)
{
if (data.Role > 8 || data.HasKey)
{
// 管理者なのでtrueを返す
return true;
}
}
return false; // 権限無しなのでfalseを返す(管理ページを開く権限なし)
}
public async Task RegistUser(User data)
{
// 新規追加ユーザーを加える
_context.User.Add(data);
// スレッドをawaitして保存を待つ
await _context.SaveChangesAsync();
}
びっくりするくらいのダーティーコードだね。まさにウンコードの中のウンコード。もしこのコードを見てなんの違和感も感じなかったあなた!、、、大丈夫!この記事を読めばしっかりわかるようになる。
ちなみに、Linq(C#の書き方)を使っていないとか、asyncいる?とかそういうことがダーティと言っているのではないのでご注意を。あくまでこのコードを正としてもっともっと読みやすく書こうよって話。
ダサいという表現はこのうんこっぷりを表せていないのだが、要は名前を見ただけで一体何をする関数なのか、どんな値が入っている変数なのか分かるような名前を付けなさいってこと。
IsAllowedAdminPageって関数のセンスは良くないかもしれないけど、多分誰が読んでも「管理画面への許可があるか」って意味だとわかると思う。もう少し短くIsAllowedAdminでもいい気がする。CheckUserってちょっと抽象的過ぎて一体何をチェックしているのかわからないし、他の関数と被りそう。ちなみに、“Is~”、“Has~"とかで始まる関数や変数は、それだけでBool(真偽)だということがわかる命名であり、広く使われている。
同じように変数の名前も見た瞬間に、それが何なのかわかる名前にしておく必要がある。例えば"i"ってよくforするときに出てくる変数名だけど、ここで出てくる変数iと被らない?たまたま今回for文がないからいいけど。userIdにしておけば、なんのidなのかすぐわかる。同じようにDBから取得したデータはUserデータなので、変数名はuserでいいでしょ。
これは非常に有名な話なんだけど、日本人が書くコードには登録を意味しようとした"regist~“がよく出てくるらしい。たしかに私も何度もそのような命名を見たことがある。しかし、***英語で登録するを表すのは"register”***なんだよね。ていうかそもそも「登録する(register)」って言葉をプログラミングの中で英語で使うこと自体なんか違和感がある。
新規で登録の場合は、"Add“とか”Create“とかを使うと誰でもわかる。もしくは追加と編集を分けたくない場合は、"Save“でも問題はない気がする。
これ経験が浅い人にマジで多いんだけど、コード見れば瞬時にわかるよっていうコメントをいちいち書く奴がいるんだよね。
「// データベースからidが一致するユーザー情報を取得する」
>うん。見ればわかるよ。
「管理者なのでtrueを返す」
>うん。だからすぐ下を見ればわかるから逐一説明しなくていいよ。
こういうコメントを見ると”この作成者って本当に頭が悪いんだなぁ“って思われるから絶対にやめておこう。また後日、もし書かれているコメントと異なる処理に変更した場合、コメントもわざわざ変更するの?そのコメントの変更を忘れていたら?とりあえずこういう単純説明型コメントは害悪でしかないってこと。一見分かりにくい処理をしている場合に、その理由などをコメントするのは意味のある正しいコメントだけど、そういうケースは多くはない。
逐一処理を日本語にしてコメントで書く必要はないけど、関数やメソッド、プロパティに対してDocコメントは書いた方が良いと思う。通常はinterfaceに対してDocコメントを書くと思うけど、今回は関数にコメントしてみる。Docコメントは以下のように、ドキュメントを作成するための情報を定義する特殊なコメントだ。今回はC#のDocコメントだけど、他の言語でももちろんDocコメントは存在し、書き方のルールがあるからググってみよう。
/// <summary>
/// 管理ページへのアクセス権限があるか否か
/// </summary>
/// <param name="userId">ユーザーID</param>
/// <returns></returns>
public bool IsAllowedAdminPage(int userId)
Docコメントを各理由は主に以下の2つだ。
ドキュメントを作成するSwaggerも非常に便利だが、2.のようにエディタ内で表示される機能はもっと身近で利便性を感じられるだろう。(以下のような表示)
Docコメントに少しでも概要を書いているとこのように外部から呼び出そうとしたとき「どんなメソッドなのか」などの情報を、わざわざDocコメントが書いている箇所まで見に行かなくても認識できるわけだ。
これは好き嫌いわかれるだろうけど、処理のネストはできる限り避けた方がいい。ネストが多いっていうのは以下のようなケースだ。こういうのって読みにくくなる原因だと思うわけ。
if (is_nest_a)
{
if (is_nest_b)
{
if (is_nest_c)
{
//なんかの処理
}
}
}
return;
以下のように書き換えた方が、断然リーダブルになると思わないか?
if (!is_nest_a || !is_nest_b || !is_nest_c) return; // 先に排除
//なんかの処理
「なんかの処理」が何十行にも及ぶ場合、この「なんかの処理」を行う必要のないものを先に排除して、あとはじっくり「なんかの処理」が続いていくって方がリーダブルだと思われる。
長い名前をつけるとキーボードで打つのがめんどくさくなるという理由で、アホみたいに変数名に"d"とか"i"とかを付ける人がいる。えーと、何、あなたサクラエディタかなんかでプログラミングしてるの?今時のエディタやIDEではサジェストされるので、むしろ分かりやすい(認識しやすい)名前をつけておいて、2,3文字入力したら候補がでてくるでしょ?もしサジェストされないような環境で開発しているなら、まずその環境を変えた方が効率的じゃないか?(致し方ないケースもあるとは思うが)
極端だけど、例えば、「GetHarryPotterAndThePhilosophersStoneDataWithUserId」っていう関数名があってもいいと思うんだよね。パッと見「ハリーポッターと賢者の石のデータを取得する関数」ってなんとなくわかるし、“GetHarr"くらいライプしたところでこの関数がすぐにサジェストされるから全部タイピングする必要なんてない。
それでは上記の修正点を反映したコードにリファクタしてみると以下のようになった。どうだろう?目に入った瞬間にどんなコードかすぐに理解できないだろうか?
無駄にコメントが書かれていないうえに命名が体を表しているので、少ない行数のコードがパッと目の神経細胞を通り、そして脳のニューロンにサッとくぐらせて、少ない消費カロリーでコードが何を表しているのか認識できるはずだ。
/// <summary>
/// 管理ページへのアクセス権限があるか否か
/// </summary>
/// <param name="userId">ユーザーID</param>
/// <returns></returns>
public bool IsAllowedAdminPage(int userId)
{
var user = _context.User.where(u => u.Id == userId).First();
if (user == null) return false;
return (user.Role > 8 || user.HasKey);
}
/// <summary>
/// ユーザー情報の新規追加
/// </summary>
/// <param name="user">保存するUserエンティティ</param>
/// <returns></returns>
public async Task AddUser(User user)
{
_context.User.Add(user);
await _context.SaveChangesAsync();
}
まとめるとリーダブルコードってのは、誰でも見た瞬間にざっくり意味がわかってしまうという美しさを持ったコードをいうのだ。前述の通り、リーダブルなコードを書くということは他人だけでなく未来の自分自身に対しても優しくできるってことだ。ひいてはプログラミング能力を大きく向上させることにつながる。英語が苦手な人、プログラミングのお作法のようなワードを知らない人(getterやsetterのような)も、はじめは間違えてもいいんだよ。大切なことは自分はリーダブルなコードを書くんだという意思を持っておくことだ。
という風に偉そうなこと言っているけど、私も「これは使い捨てのスクリプトだから」みたいな言い訳で、変数名にretとかtmpとかを付けてしまうことは今でもよくある。(笑)
要するに気張らずにできる限りリーダブルを意識しましょうってこと。
そう。私は以下の書籍に影響を受けています。もっとたくさんの学ぶべきことが書かれている書籍なので絶対にお勧め。
前回の記事では、どんな初心者でも「投票システム(フロント)」を実装できるっていうことを紹介したんだけど、当然ググってコピペ、そのコードをひたすらコピペで量産では、ごみコード(またの名をウンコード)になるのはいうまでもない。なので前回の状態からレビューをしてくれる架空の先輩エンジニ …
前回の記事で紹介したプログラミングの思考プロセスを使って、実際に何かを組んでいく様をお見せする。この思考のプロセスは基本的に汎用性があるので、真似してくれたら間違いなく上達する。前回も書いたがこの記事は、なかなか上達しないと悩んでいる初心者をターゲットにしているのでご注意を。 今 …