#0. 今回お話しすること
- 「メンテナンスしやすいコード」という軸でまとめたよ
- 心構えの部分を少しと、具体的なテクニックを紹介するよ
- Javaやjavascriptコードで説明を書くけど、全言語共通で使えるテクニックだよ
- 私個人の考えなので、「いいな」と思うものだけピックアップしてもらえたら何よりです
#1. はじめに
今回は、よりよいコードの書き方について、私の考えをまとめました。
連載モノにする予定なので、今回紹介するのはごく一部です。
コーディングに100%の正解はありません。
なるべくメリット／デメリットを挙げた上で紹介しますので、ときと場合に応じて使い分けるのがいいでしょう。
なお、本記事は「今後維持メンテナンスし続けるシステム」を想定しています。
「多少他人にわかりづらくても、コード量やコーディング速度を重視するシステム」にはおすすめしません。
#2. 基本的な心構え
新人エンジニアが一皮むけてくると、こんな考えに至ります。
- 先輩やみんなが知らないような処理方法を見せつけてやろう
- 僕にしか思いつかない天才的なアルゴリズムでびっくりさせよう
これ、誤解を恐れずに言えば、やめたほうがいいです。
「先輩やみんなが知らないような処理方法を見せつけてやろう」という方から補足します。
新しい技術をどんどん取り入れていくことは大歓迎です。ただし、これと必ずセットで考えてほしいのは、ほかのメンバーへの情報共有・教育です。本人がどんどん成長していくことはうれしい限りですが、そのコードをレビューできる人がいないというのは組織としてまずいです。切り開いていく人と、教育する人は必ずしも同じ人でなくてもいいですが、必ずペアで行いましょう。
「僕にしか思いつかない天才的なアルゴリズムでびっくりさせよう」という方については、ほかの人があなたのコードを理解できないならやめておきましょう。「へー、なんかよくわからんけど、●●さんが作ってるし、結果も正しいもの返ってきてるし、いいんじゃない？」は組織としてだめです。多少長くても、多少リソースを無駄に使っても、「維持メンテナンスしやすいコード」を優先すべきです。目安としては、メンバーのうち8割くらいの人がコードを見て少し考えたらわかる、くらいでしょうか。もちろん、残りの2割のために教育は挟んでいきましょう。
#3. テクニック集
##3.1. 何も考えず、引数にオブジェクトを渡すのはやめよう
public class Item {
// ユニークなアイテムID
private int id;
// アイテムの名前
private String name;
}
// アイテムを削除する
public void deleteItem(Item item) {
// 削除処理
}
上のコードは、すでにDBに登録されているデータがあり、ブラウザからキーとなるidを受け取って削除する処理を想定しています。ふつう、削除処理を実行するために必要なものは何でしょうか。そう、何を削除するかを一意に示す「id」だけがあれば十分ですよね。
ですが上の例ではidだけをメソッドに渡すのではなく、Itemオブジェクトとして渡しています。
今後このdeleteItem()を別のところで利用したくなった場合、わざわざItemオブジェクトをnewして渡す必要があります。
またそのとき、nameには値を入れないといけないのでしょうか。
使わないからnullのまま渡すのだとしたら、今後の改良で誤ってnullのnameを利用してしまうことはないのでしょうか。
色々と考慮すべきことが出てきてしまいます。
public class Item {
// ユニークなアイテムID
private int id;
// アイテムの名前
private String name;
}
// アイテムを削除する
public void deleteItem(int id) {
// 削除処理
}
こうしておけば、「あー、削除処理にはidだけあれば十分なんだな」ということも自明ですし、かつ再利用しやすくなりますね。
しかし、今後の改良で「削除履歴機能を付けたいから、何を消したかわかるようにnameの値も削除時に必要だ！」となったとします。そうなるとdeleteItem()のインターフェースが変わるため、全呼び出し元の修正とテストが必要になります。こういった改良があることがわかっていたのなら、あえてItemクラスを渡すのも手かもしれませんね。
また、引数の数がとても多い場合は、再利用性の低下と天秤にかけたうえで、可読性を重視してItemオブジェクトごと渡す選択もあり得ます。
######メリット
部品の再利用がしやすい
処理にはどのパラメータが必要なのかわかりやすい
######デメリット
修正が予定されている場合、利用箇所すべての修正と再テストが必要になる
##3.2. 文字列を切るときは、切り方に気を付けよう
function jumpToEditPage() {
var urlStr = (isCreate) ? "Create" : "Edit"
var url = "https://www.hoge.com/item" + urlStr;
location.href = url;
}
入力フォーム画面に遷移するときのjavascriptコードです。
何らかの条件を見て、新規登録なら"itemCreate"、編集なら"itemEdit"というurlで画面遷移したいとします。
このコードではなるべく共通化しようと、両者で異なる"Create"、"Edit"という文字列だけで分岐を発生させています。
ところで皆さん、「サービス終了したメニューだから、どこからもこの画面に遷移してこないことを確認したうえでプログラムを削除しよう」ってことないですか？そのとき、みなさんはどうやって「この画面に遷移しないこと」を確認しますか？
"itemCreate"や"itemEdit"っていう文字列でhtmlを検索しませんか？
となると、この書き方ではその検索に引っかかってこないことになります。「あ、ここはそのままじゃ引っかからないから、検索のとき注意してね！」と気付いた方がいたとしても、"Create"検索したら関係ないところが膨大な数引っかかって、結果重要なところを見落とした、ってなことにもなりかねません。
function jumpToEditPage() {
var urlStr = (isCreate) ? "itemCreate" : "itemEdit"
var url = "https://www.hoge.com/" + urlStr;
location.href = url;
}
分岐するにしても、この単位でまとめたほうがメンテナンスしやすそうですね。
######メリット
メンテナンスしやすい
######デメリット
ほぼないと思います
##3.3. 深い入れ子のif文はやめよう
public boolean test(){
if(A == a){
if(B != b){
if(C == c){
return true;
}
}
}
return false;
}
この例では3階層ですが、これが10階層とかになって、しかも条件式が長くなったりすると、もう手に負えません。また右にインデントされていくことで改行が発生し、可読性が悪くなる恐れがあります。
public boolean test(){
if(A != a){
return false;
}
if(B == b){
return false;
}
if(C != c){
return false;
}
return true;
}
どうですか？
日本語で言うと、修正前は「これで、かつこれで、かつこの時にはtrueなのね」という読み方です。全ての条件をバッファにため込んで判断するイメージです。
修正後は、「このときはこうなのね。このときはこうなのね。このときはこうなのね。それ以外はこうなんだね。」という読み方です。１つ１つの条件を順番に処理することで、わかりやすいコードに見えます。かつ階層も深くならず、可読性も低下しにくそうです。
######メリット
可読性の低下が抑えられる
######デメリット
慣れない方には読みづらいかも？
今回は以上です。
また同じような目線でまとめようと思います。