#ことの始まり
私は会社に入ってから本格的にプログラミングを始めたのですが、
1年目のコードを見返すとそれはそれはひどいクソコードを量産しておりました(いまもするのですがw)
そして最悪なことにその頃は人がおらず1人で新規アプリを開発していたので、
レビューを誰からも受けずに、そのクソコードのせいで後から開発した人に迷惑をいろいろかけてしまいました。
そのアプリ開発が終わってから、運良く優秀な先輩に1年間レビューされる機会があり色々なことを教わって、
どうしようもないエンジニアからしょっぱいエンジニアぐらいにはなったと思っています(多分)
その時のありがたい教えをもとに、過去のどうしようもないエンジニアだった自分と比較して、
こんなことを考えるのがまずはエンジニアとして最低ラインだ、思ったことが1つあったので書きます。
※objective-cをベースに書かせて頂きます
#いかにコードを読む側の考える時間を短くするか
「いかにコードを読む側の考える時間を短くするか」
今回私がお伝えしたいのはこれです。
ここを常に意識して考えるようになればどうしようもないエンジニアから卒業出来るのではと考えています。
いいコードとは何か?と考えた時に、色々な要素が出てきます。
可読性、可用性、保守性、拡張性…。
それぞれを満たすためのやり方や考え方は色々あると思うのですが、
「いかにコードを読む側の考える時間を短くするか」という視点がまずは非常に重要なのではとレビューを受けている中で思いました。
これは共同開発だけじゃなく、個人開発でもこの視点は重要だと思っていまして、
「コードを読む」のは他人だけではなく自分もだからです。
改修しよう!と思った時に、「なんでこうしているんだっけ?」と考えるようなコードを
残していないのがどうしようもないエンジニアから卒業している時なのだと思います。
##読む側の時間を使ってしまう悪い要素
こんなのが諸悪の根源です。
###しょっぱい命名
・何のクラスかわからないクラス
何のために存在するクラスなのか名前だけで一発でわからないと使う側も迷います。
「ModelController」とか何のことやねんって感じです。
何のモデルクラスなのかをはっきり書かないといつ使うのかすらもわかりません。
・見慣れない関数名
よくプログラミングであるあるな動詞ってありますよね。
- is
- has
- check
- add
- delete
などなど…。
この辺りのあるあるを使わないと「ん?」となることが多くなります。
対義語の関係性やdeleteとremoveのような似ているけど微妙に違うものを適切につけることも重要です。
ただプログラミングを始める時にまずこれらを覚えるのは面倒なので、
私はよくcodicというサイトを使っています。
https://codic.jp/
名前は非常に重要な要素だと思います。
良い名前をつけれるエンジニアになると一歩成長すると思います。
(リーダブルコードをまず読め、といわれるのも納得です)
###無駄なものが書かれているヘッダーファイル
外部に公開すべきものを厳選するのは非常に重要ですよね。
そのクラスを使う時にプロパティが参照できれば使っていいと考えます。
しかしもしそれが本当に使っていいものではなくバグを生めばバグを追うのも大変になります。
「クラスの責任範囲」といった視点でも、何を公開して何を守るのかは常に考えなければいけないと思います。
なんでこれが公開されているんだろう?と後から見返して思わないように
常にチェックするようにしています。
###中身を知らないと使えないクラス
昔こんなコードをよく書いていました。
-(void)didTapButton {
BookmarkViewController *b = [BookmarkViewController alloc] init];
b.bookmarkData = self.data;
b.isEditMode = NO;
b.hogehoge...
}
これだとクラスを使う側はBookmarkViewControllerにどんなプロパティがあるか知らないといけないし、
何があるのか調べるコストも何を渡せばいいかもわかりません。
もしいま自分が書くとしたらこんな感じですかね、、
-(void)didTapButton {
[BookmarkViewController show:self];
}
ただ呼び出すだけでいいようなメソッドにします。
責任範囲もしっかり切り分けることで、あくまで使う側は呼び出すだけ、にします。
こうすれば考える時間も減ると思います。
###深い深いネスト
あるあるですが、1つの関数の中がめちゃくちゃ長いと考える時間がそれだけ増えます。
特にライフサイクルの中身は常にシンプルに書くことを心がけています
-(void)viewDidLoad {
[super viewDidLoad]
[self p_setupTutorial];
[self p_setup...];
[self p_hogehoge];
...
}
だらだら書くよりも、何をしているのか?だけすぐにわかるように書いた方が良いですよね。
どこが原因かも特定しやすいのがメリットだと思います。
###長い条件式
if文の中の条件が増えるとそれだけでもう頭を使うと思います
-(void)addBookmark:(BookmarkItem *)item {
if(item.url != nil && item.title != nil && item.hogehoge != ...)
...
}
外部からの入力は心配性なぐらいになるのは良いのですが
こんなのがいくつもあると読む側は辛いので
-(void)addBookmark:(BookmarkItem *)item {
if([self isValid:item])
return;
...
}
こんな感じにかけると、わかりやすいかなと思います。
###「どうしてこう動くのか説明できないコード」が存在する
OSのバージョンによってViewの扱い方が変わったり、
特定端末での不具合で処理を変えることはありますよね。
書いている時は必死なので、「よし!動いた!」でコミットしてプッシュしちゃうことが私もありました。
しかしこれでは、他の人が見た時に「ん?なにこれは」となっていちいち調べなくてはいけません。
適切なコメントや、参考にしたドキュメントがあるならそれを貼ると迷わなくなるのでおすすめです。
- (NSURL *)getImageURL {
// iphone5ではhogehogeな理由でURLが取れないため。
if([DeviceUtil isIphone5])
return nil;
}
@TSKGunGunさんのコメントから書かせて頂きました、ありがとうございます!
無駄に長いコメント、いらないコメント
それ書く意味ある?となるようなコメントや、3行以上あるコメントは
余計に見る側が頭を悩ませてしまいます。
- (NSURL *)getImageURL {
// iphone5は処理しない
if([DeviceUtil isIphone5])
return nil;
}
このコメントとかは処理と同じことを書いているだけなのでいらないですよね。
僕の先輩の大好きな言葉「code is real」に反するやつですね。
長々とコメントをつけたり補足するぐらいなら、より短くシンプルに関数名を書くほうが得策かと思います。
@TSKGunGunさんのコメントから書かせて頂きました、ありがとうございます!
###マジックナンバーが大量にある
なんの数字かわからない数字がたくさんあるとそれだけ頭を悩ませますよね。
-(BOOL)isAuthenticate {
...
if(!result) {
if(error.code == 1)
return YES;
return NO;
}
}
これだと、error.codeの内容が何かわかりません。
なんのエラーだと例外的にYESにするんだろう?と思いますよね。
-(BOOL)isAuthenticate {
...
if(!result) {
if(error.code == TouchIDNotEnrolled)
return YES;
return NO;
}
}
これだと、なんのエラーかわかりやすくていいですよね。
@koitaroさんのコメントから書かせて頂きました、ありがとうございます!
###フレームワークに沿っていないクラス設計
例えばですがMVCを採用しているのに責任範囲が適切になっていないとそれだけ迷います。
モデルクラスにあるべき関数などはある程度テンプレがあると思うので
そこに従って基本は作ると「ああ、はいはい」と読む側も理解が早くなると思います。
#他にも色々あるのですが
ほとんどの場合において、「いかにコードを読む側の考える時間を短くするか」
という視点が入ると思います。
もちろんクラッシュしない、処理の高速化などの視点も非常に重要だと思うのですが
より一歩良いコードをかけるようになるための根底として、ここが自分はなかったなと振り返って思います。
最近コードをレビューする機会も増えたのですが、この視点を持つとレビューもしやすくなりました。
最後に
私自身、まだまだ出来ないエンジニアだとは思うのですが、
まずはどうしようもないエンジニアから抜ける一歩としていつまでも忘れないようにしようと思っています。
もっとこんなことも考えなくちゃだめだぜ!などあればぜひコメントください!