メンテナンス性の高いコードを残すために意識していること
概要
こんにちは。バックエンドエンジニアのfuです。
最近はプレイングマネージャーとしてメンバーの技術的なリードやレビューをする機会が増えているため、自分の方針がブレないように自戒の念を込めてメンテナンス性の高いコードを残すために意識していることを記載しておこうと思いました🙋♂（レビューのたびに違うことを言っていては信頼されないので...）
普段は主にバックエンドでTypeScriptを記述することが多いので、今回はその前提で自分が意識していることを書いていこうと思います。
ただ、自分自身フロントエンド開発に入ることも多く意識していることはほとんど同じです。
※こちらの記載内容は自社の社内システムの開発を主に行なってきた経験をもとに記述しているため、受託開発やSaaSの開発だとちょっと違うな。。。と思うかもしれませんのであらかじめご了承ください🙇♂️
マインド編
まずはマインド面についてです。設計やコーディングに入る前にどういう意識を持って取り組んでいるかについて記述しておきます。
1.仕様を把握していない第三者が読んで迷わないかを意識する
私はいつも、「今参画しているメンバーが全員辞めて他の人が見るようになった時に理解ができるか？」という観点を持ってコードを書いています。
ここでポイントなのは同じチームの他のメンバーではなく全く関係ない別の人が見ても迷わないというところです。
極端ではありますが、そのくらいの意識を持って書いていないと意外と他の人が書いたコードって読みにくくなってしまうと思っています。
引き継ぎのないまま前任者が辞めて、誰も仕組みが分からずメンテできなくなっているシステムの担当になることもあったので、自分の持っているプロダクトは絶対そんな状態にはしないぞという思いでこの意識を続けています...😂
2.既存のコードが正ではない
私は基本的に既存のコードは疑って見ています。
悪い意味ではなく、そのような観点を持っていると自然とコードが綺麗になっていくと考えています。
機能追加をする時に既存のコードを参考にしてコピペしたものを修正するっていうことはよくあると思うんですが、その時に
「このコードこう書かれてるけどこっちで書いた方が記述量が減るしロジックもシンプルにならない？」
と気付いてリファクタリングするっていうことも度々あります。
3.HRTの原則を忘れない
こちらはコーディングというよりレビューする上で意識していることですが、「Humility（謙虚）」「Respect（尊敬）」「Trust（信頼）」を常に忘れないようにしています。
例えば
- 「こちらの書き方の方が可読性が高いので修正お願いします。」
- 「最近はこのような書き方は一般的ではないのでやめましょう。」
みたいな感じでレビューをしても、レビューイが嫌な気持ちになるだけではなく他の人の考え方を取り入れることができず自分の成長機会を損失してしまうと考えています。
- 「こちらの書き方だと記述量が減って可読性も上がると思うんですがいかがでしょうか？」
- 「こちらの処理を使う方が適していると思っていたんですが、どのような意図でこの処理を追加しましたか？」
こちらのように聞かれる方がレビューイも意見を言いやすいし、レビュアーの考え方が適切だった場合に次からはそうしようって思えるんじゃないかなーと考えています😁
そもそも自分のコードが絶対に正しいとは微塵も思っておらず、常に他の人の考え方を吸収していきたいですね👍
4.仕様を理解しないままレビューしない
こちらもレビューする上で意識していることですが、コードの記述方法だけを確認するのではなく、仕様を理解した上でそれが本当に最善のコードかを判断するようにしています。
- 「書き方的には綺麗で問題ないんだけど、この要件だったらこの判定だけで十分では？」
- 「他の記述を参考にして書いていると思うけど、この処理には不要なロジックが混ざってしまっていない？」
といったことに気づくことができるようになります。
ここで意識しているのは
仕様通りに書くことができているかというのはもちろん、その仕様だったらもっといい書き方がないか？
という観点です🙋♂
5.業務知識をおろそかにしない
私は質の高いコードを書くためには業務知識を正確につけることが必要不可欠だと考えています。
ユーザーからの要望があった際に、業務知識がなければ言われた通りの機能しか追加することができませんが、正しい知識があればよりユーザーにとってプラスとなる機能の提案や、要望を満たすために必要十分でメンテナンス性の高いコードの記述ができるようになります。
設計編
1.YAGNIの法則
YAGNIとは「You Aren't Going to Need it」の略で、今必要でなければそれはいらないよねみたいな考え方です。
- 「後々機能拡張する時に備えて引数多めに受け取る設計にしておこう」
- 「今は使われてないけどこのケースも網羅しとくほうが便利だから入れておこう」
みたいなケースは度々あると思うんですが、経験上それが使われることってほぼないと思っています。
むしろ無駄な考慮を含んだ設計にしたまま前任者が辞めて、残されたメンバーは全く使われていない謎のコードに悩まされるケースの方が多いと考えているためYAGNIは基本方針としています。
2.拡張・削除・テストがしやすいか
私はクリーンアーキテクチャやオニオンアーキテクチャの考え方が好きで、メンテナンス性の高いコードを書くためにはそのような概念の良いところを積極的に取り入れたいです。
拡張・削除・テストがしやすいということはクリーンアーキテクチャでいうところの責務が明確になっているかという考え方に近いのではないでしょうか。
ここでいう拡張しやすさとはYAGNIの法則のところで話した余計な考慮を入れておくということではなく、機能追加や改修が行われた際に仕組みを壊さずに最小限のコード量で拡張ができるかという観点です。
例えば
- 一つのfunctionにロジックを詰め込みすぎていて、改修しようと思ったらそのfunctionの前提が変わって大きな変更が生じた。
- 別のClassが変更対象のClassにかなり依存しているため、変更を入れると影響範囲が計り知れない
といった場合は拡張しにくいですね。
Classやfunctionの責務を明確に分けて依存しすぎないようにすることで拡張しやすいコードになっていき、そうすると自然とそのClassやfunctionは削除がしやすくなり、テストもしやすくなります。
3.適切な箇所で処理を行なっているか
こちらはClassごとの責任的な意味で、Cotroller、UseCase、Repository、Serviceなどアーキテクチャによって定められたそれぞれの責務をちゃんと全うできているかという考え方になります。
例えば
- DBアクセスは必ずRepositoryで行うようにする
- UseCaseにEntityで行うべき処理が実行されないようにする
みたいな感じです。
それぞれが持つ責務を適切な箇所で行うことによって、不具合が発覚した時でも実行箇所の特定が容易になり、バグの早期発見・スピーディな対応に繋がっていきます。
ただし、こちらはチーム内でしっかりと認識を合わせ適切にレビューを行わないと簡単に崩れてしまうとも考えているため、新規メンバー参画時は理解し、納得してもらえるまで入念に説明とすり合わせを行なっています。
4.ユーザーの要望を100%満たすべきかを考える
こちらは私が社内システム開発に携わっているから言えることかもしれないんですが、ユーザーの要望を100%満たすべきかというのは常に考えるようにしています。
経験上、ユーザーはシステムがどのような仕組みで動いているのか完全に把握していることは滅多にないため、自分達の業務を遂行するという観点のみで機能の追加要望が上がってきます。
そのようなケースで私は、その要望を満たす機能追加をした際に、システムとしてのメンテナンス性はどうなるかということにも注目しています。
例えば極端に言うと、
「ユーザーの要望を100%満たすように書いたらDBのカラム追加もたくさん必要で多分自分がいなくなったら誰もメンテできないな」
と言うような場合には
「ユーザーの要望を完全には満たしていないけど機能拡張もしやすいし何かあった時自分がいなくても対応できるような設計」
を優先するケースもあります。
システムはできて終わりではなく、これから先長い付き合いになるのでそのことも考え、ユーザーに理解してもらった上で良好な関係を築いていきたいですね😄
5.JSDOCをちゃんと書く
設計というよりコーディングかもしれないですが、JSDOCを有効に使ってそのfuctionでやりたいことを正確に伝えることを意識しています。
ここでは処理ごとのコメントというよりfunctionごとの説明のことを指しており、コメントだと人によって量や質に差が出やすいですが、「fucntionを書くときはそのfunctionの概要を説明したJSDOCを書く！」というルールを決めておけばチームに浸透しやすく、形式も揃えやすいです。
コーディング編
次はコーディングする際に意識するポイントについてです。
リーダブルコードに記載されているようなことも一部あるかもしれませんがご了承ください🙇♂️
1.変数名で何を表しているのかをわかるようにする
# good
# functionサンプル
const getFavoriteFoodNamesByAge = (age: number) => 年齢をもとに好きな食べ物を取得する処理
# valueサンプル
const isSpecialStatus = 特別なステータスかどうかの判定
# bad
# 中で何が行われているのかわからない
const getFoodName = (age: number) => 年齢をもとに好きな食べ物を取得する処理
# 実際はbooleanなのにステータスそのものが返ってくると想像してしまう
const specialStatus = 特別なステータスかどうかの判定
自然と意識されている方も多いと思いますが、変数名に意味を持たせることを意識しています。
- 例えばgetFavoriteFoodNamesByAgeという関数を呼び出していた場合、呼び出し先まで行かなくても「年齢をもとに好きな食べ物の一覧を取得しているんだな〜」というのが分かる。
- 返却される値が複数形の場合は「FoodNames」というように複数形の変数名をつけ、関数の場合は「get~」「create~」でbooleanだったら「is~」というように中で行うこと・返却されるものを的確に表現する。
経験上、変数名が長くなってもその役割を明確に示している方が後々役立つ場面が多いと考えています。（変数名が長すぎる場合は役割を持たせすぎているので分割した方がいい可能性もありますね）
2.内部functionの中に責務を閉じ込める
# good
const getFavoriteItems = () => {
const getFavoriteFoodNames = () => {
const foodNames = ...
const favoriteFoodTypes = ...
...好きな食べ物を取得する
}
const getFavoriteSportNames = () => {
const ｓportNames = ...
const favoriteSportTypes = ...
...好きなスポーツを取得する
}
...
return {
favoriteFoodNames: getFavoriteFoodNames(),
favoriteSportNames: getFavoriteSportNames(),
...
}
}
# bad
const getFavoriteItems = () => {
# 好きな食べ物に関する情報を定義していく
const foodNames = ...
const favoriteFoodTypes = ...
const favoriteFoodNames = ...
# 好きなスポーツに関する情報を定義していく
const ｓportNames = ...
const favoriteSportTypes = ...
const favoriteSportNames = ...
...
return {
favoriteFoodNames,
favoriteSportNames,
...
}
}
こちらは設計の時にもあった「責務が明確になっているか」という考え方と似ています。
上記のgoodの例のメリットとしては
- getFavoriteFoodNames内部で使われている値がそのfuctionでしか使われていないことが明確になっている
- getFavoriteFoodNamesの仕組みに改修が入った際に、他の処理への影響範囲を容易に把握することができる
などがあげられます。
badの例だとfavoriteFoodNamesの取得のために定義したvalueをfavoriteSportNamesを取得するために使うことも可能になっており、影響範囲が特定しにくくなってしまいます。
コード記述量は増えますが、JSDOCを内部fuctionに書けば仕様の把握もしやすくなりますね。
3.functionやClassの引数の型には必要な項目だけを定義する
# 各種データ取得項目の型
type DataType = {
id: number
foodNames: string
sportNames: string
season: string
}
const getFavoriteItems = () => {
# good（引数の型を必要な項目だけにしている）
const getFavoriteFoodNames = (args: {
foodNames: string[]
season: string
}) => {
...好きな食べ物を取得する
}
# bad（引数の型に全ての項目を要求している）
const getFavoriteSportNames = (args: DataType) => {
...好きなスポーツを取得する
}
const data: DataType = getData() # 各種データを取得する処理
const favoriteFoodNames = getFavoriteFoodNames(data)
const favoriteSportNames = getFavoriteSportNames(data)
...
}
ここでやっていることはfunctionの期待する引数に、その中で使うものだけを定義しているというものになります。（内部functionに限らず他でも活用できます）
こうすることのメリットとしては
- 処理内でどの項目が使われているかが明確になり仕組みが理解しやすくなる
- fuctionのテストをする場合、引数に不要な項目を含めなくて良くなる→結果的にテストも見やすくなる
などが挙げられます。
ただ、こちらは場合によっては引数に丸ごと渡した方がわかりやすかったり、型を別で定義することで管理する型が増えてしまうなども起こりうるので、状況によって的確に判断できれば良いと考えています。
4.早期リターンを活用して可読性を上げる
# good
const getRankByPoint = (point: number) => {
if (point == null) return null
if (point < 60) return 'C'
if (point < 80) return 'B'
if (point < 90) return 'A'
return 'S'
}
# bad
const getRankByPoint = (point: number) => {
if (point == null) {
return null
} else if (point < 60) {
return 'C'
} else if (point < ８0) {
return 'B'
} else if (point < 90) {
return 'A'
} else {
return 'S'
}
}
こちらは好みの問題もあるかもしれませんが、早期リターンをすることによってコードがシンプルになり視認性が上がると考えています。
上記は簡単な例なので「switchで書いた方がいいだろ！」と考える方もいるかもしれませんが、switchは判定する項目と判定する条件の距離が遠くなって記述量が多いと追いにくくなるのでif文で早期リターンする形の方が個人的には好きです😄
こちらに関してはチーム内で書き方を統一すればいいと思っています！
5.その他
この他にも
- 2箇所以上に共通で使われている処理はできるだけ外出しする
- 処理がネストしすぎている場合は内部functionの追加や早期リターンを活用する
- テストを作る際は責務ごとに処理を網羅した適切なケースを作成し、仕様に依存させない
など色々と意識してコードを書くようにしていますが、あげていくとキリがないので厳選して残しておきました。
新たな気づきがあれば追加していきます。
終わりに
ここまでお読み頂きありがとうございました。
以上がメンテナンス性の高いコードを残すために私が意識していることです。
これからもっとメンテナンス性の高いコードを残していけるように、時々こちらの内容を振り返りながらより良い手法を見つけられたらと思います。
こんなのもおすすめだよ！という意見があればコメントくださると幸いです。
Discussion