「良いコード」を書くために意識している17のTips まとめ
この記事について
私はWeb基盤を提供している企業でWebアプリケーションエンジニアをしています。
インフラや顧客基盤など複数のバックエンドAPIが動いており、それらを結合したアグリゲーション（BFF）の作成とフロントエンドの実装を担当することが多いです。
言語はTypeScriptとC#を選択する事が多く、フロントエンドではReact.jsとVue.jsを使用しています。これらをコンテキストとして私が「良いコード」を書くために日頃意識しているTipsを投稿していきます。
やらないこと
- インデントを揃える、命名基礎、アクセス修飾子などの基礎的な内容は割愛しています。
- コードはTypeScriptで書いています。ですが特定の言語に特化した内容にはしていません。
- プログミング全般に掛かかった内容であり、フロントエンドやバックエンドなど特定のレイヤーに特化した内容にはしていません。
良いコードとは
ここで言う「良いコード」の定義は以下とします。
- 保守性が高い
- 可読性が高い
それぞれの意味をwikipediaで調べました。
保守性
バグなどの不具合要因を修正したり，性能や使い易さといった特性を改善したり、製作当初に想定していなかった機能の追加や変更を少ない手間やコストで実施・改変が行える容易さを表す。
可読性
プログラムのソースコードを人間が読んだときの、その目的や処理の流れの理解しやすさを指している。
保守性は可読性を包括した考え方だと思いますが、敢えて分けさせていただきます。
目次
- コメントは背景から書く
- 要約変数を使う
- 説明変数を使う
- モジュールに切り出す
- 早期リターンする
- 横断的関心事はAOPで外出しに
- 不変型（immutable）を使う
- カプセル化
- レガシーコードを隠蔽する
- 凝集度と結合度
- 継承は基本的に使わない
- 無理に共通化しない
- Dependency Injection（DI）
- Dependency Inversion Principle(DIP)
- ジェネリクスを使う
- アーキテクチャーを参考にする
- 自動生成に頼る
コメントは背景から書く
以下のように、実装をただ日本語化しただけのコメントをたまに見かけます。
// masterロールまたは、isOldTypeAccount=trueかつ、adminロールの場合に実行
if (user.role === "master" || (user.isOldTypeAccount && user.role === "admin")) {
// 管理者ユーザーのみに許可される処理
}
これではisOldTypeAccountとは？ adminロールとは？ 以降の修正でも考慮する必要ある？など不安や疑問が残ります。
// 管理者ロールのみ実行
// サービスリニューアル以前のアカウント（isOldTypeAccount=true）は管理者ロールにadminを使用していた
if (user.role === "master" || (user.isOldTypeAccount && user.role === "admin")) {
// 管理者ユーザーのみに許可される処理
}
背景を記述する事で、なぜ「masterロール」と「adminロール」が混在しているのか。
なぜisOldTypeAccountフラグが作られたのか、その理由がクリアになりました。
実装者と読み手では持っている知識の前提が違います。そのことを考慮してコメント（や実装）をするべきでしょう。
要約変数を使う
先程使用したコードから
サービスリニューアル以前のアカウント（isOldTypeAccount=true）は管理者ロールにadminを使用していた
の部分を要約変数にします。
const isOldTypeMasterRole = user.isOldTypeAccount && user.role === "admin";
if (user.role === "master" || isOldTypeMasterRole) {
// 管理者ユーザーのみに許可される処理
}
上記のようにすることでコメントも不要になりました。モジュールに切り出すと尚、良いでしょう。
if (isMasterRoleUser(user)) {
// 管理者ユーザーのみに許可される処理
}
説明変数を使う
目的の値を取得するために、クラスの深い階層を参照したり、危険な文字列操作を行うことがあります。こういったレスポンスを愚直に処理しても見栄えが悪く、可読性が低下します。
class UserGetService {
public get(userId: string) {
const api = new LegacyApi()
const resp = api.getUsers(userId)
return {
userId: resp.data.users[0].id,
lastName: resp.data.users[0].info.fullName.split(" ")[0],
firstName: resp.data.users[0].info.fullName.split(" ")[1],
}
}
}
この場合、説明変数を定義することで可読性の向上が見込める可能性があります。
class UserGetService {
public get(userId: string) {
const api = new LegacyApi()
const resp = api.getUsers(userId)
const user = resp.data.users[0]
const userNames = user.info.fullName.split(" ")
const lastName = userNames[0]
const firstName = userNames[1]
return { userId: user.id, lastName, firstName }
}
}
モジュールに切り出す
ユーザー作成のモジュールを例にします。
悪い例ではuserId passwordに対するバリデーションの処理がノイズとなり可読性が低下しています。
function createUser(userId: string, password: string) {
// userIdのバリデーション
if (userId && userId.length >= 8 && userId.length <= 32) {
// passwordのバリデーション
if (password && password.length >= 8 && password.length <= 64) {
const api = new UserApi(userId, password)
api.create(userId, password)
}
}
}
userIdを例にすると下記3つのチェックを行っています。
-
userIdに値があるか -
userIdが8文字以上か -
userIdが32文字以内か
これらは「userIdが想定通りの値になっているか」という関心事を処理しています。関心事が一致している場合は別モジュールとして切り出すことができます。
function createUser(userId: string, password: string) {
if (isUserIdValid(userId)) {
if (isPasswordValid(password)) {
const api = new UserApi(userId, password)
api.create(userId, password)
}
}
}
function isUserIdValid(value: string) {
return value && value.length >= 8 && value.length <= 32
}
function isPasswordValid(value: string) {
return value && value.length >= 8 && value.length <= 64
}
モジュールに切り出し、命名することでどのような関心事を処理しているのか一目で理解できようになりました。コメントにより処理を補足する前に、その処理はモジュールとして切り出すべきでないか考えると良いでしょう。
早期リターンする
早期リターンを使用するメリットは何よりネストを軽減できるところにあります。ネストが深くなると可読性が低下します。
function createUser(userId: string, password: string) {
if (isUserIdValid(userId)) {
if (isPasswordValid(password)) {
const api = new UserApi()
api.create(userId, password)
}
}
}
処理の本質でないところ、例えばAPIの実行やDBに記録する前のバリデーションなどは早期リターンするべきでしょう。
function createUser(userId: string, password: string) {
if (!isUserIdValid(userId)) { return }
if (!isPasswordValid(password)) { return }
const api = new UserApi()
api.create(userId, password)
}
悪い例に比べ、良い例ではネストが軽減され、本質的なコードの見通しが改善されました。
横断的関心事はAOPで外出しに
言語によって呼び方が変わりますが、Javaでいうアノテーション、C#でいうアトリビュート、JavaScriptでいうデコレーション…の使用を推奨します。
例えばユーザー作成のモジュールに、認証やログ出力の処理などが混ざると、そのクラスの関心事に対してノイズとなり、可読性が低下してしまいます。
こういった処理は外出しにし、本質的な処理に集中できるよう読み手に配慮するべきでしょう。
function createUser(userId: string, password: string) {
this.logger.log(Level.info, "start user create")
if (!this.auth()) { throw new UnauthenticatedError() }
if (!isUserIdValid(userId)) { return }
if (!isPasswordValid(password)) { return }
const api = new UserApi()
api.create(userId, password)
logger.log(Level.info, "end user create")
}
@auth()
@log("user create")
function createUser(userId: string, password: string) {
if (!isUserIdValid(userId)) { return }
if (!isPasswordValid(password)) { return }
const api = new UserApi()
api.create(userId, password)
}
不変型（immutable）を使う
公開しているフィールドが変更可能だと、それが生存している間、どこかで変更されてないか常にwatchしておく必要があります。
変更不可の状態で宣言することで、ワーキングメモリ（作業や動作に必要な情報を一時的に記憶・処理する能力）が解放されて可読が容易になります。
class User {
- public userId: string
+ public readonly userId: string
}
プライベートなフィールドもなるべく不変型にするべきです。
「書き換え不可」という情報は、コード量が多くなればなるほど効果を発揮します。
class User {
public readonly userId: string
- private password: string
+ private readonly password: string
}
コンストラクタによってクラスの初期化を制限できる言語では、コンストラクタの引数以外でそのクラスの状態を変更できないことが最も望ましいです。
「フィールドはコンストラクタでしか変更できない」という情報があれば、そのインスタンスは膨大なコードの中に埋もれても、手放しで再利用することができます。
class CreateUserCommand {
public readonly userId: string
private readonly password: string
+ constructor(userId: string, password: string) {
+ this.userId = userId
+ this.password = password
+ }
public execute() { ... }
}
引数が参照型の場合は、引数も不変型にすることが望ましいです。
これにより、メソッドの内部で引数が変更されない（破壊的メソッドでない）ことが保証され、メソッドの内部を確認する必要がなくなり、カプセル化が強化されます。
class CreateUserCommand {
public readonly userId: string
private readonly password: string
- constructor(user: { userId: string; password: string }) {
+ constructor(user: Readonly<{ userId: string; password: string }>) {
this.userId = user.userId
this.password = user.password
}
public execute() { ... }
}
不変型をまとめます。
- フィールドは
readonlyにする -
setterを公開しない（外部からの変更を許可しない） - コンストラクタの制約を利用する
- 引数も不変型で宣言する
カプセル化
カプセル化の解釈は人によって違うかもしれません。
私はカプセル化を「実装を確認しなくても正しく使用できること」だと思っています。
「実装を確認しなくても正しく使用できる」とはクラス定義を見るだけで、どのようなモジュールか理解できるということです。
class Register {
public userId: string
public password: string
public execute() {
if (!this.userId) { throw Error("userIdに値を入れてください") }
if (!this.password) { throw Error("passwordに値を入れてください") }
// ユーザー作成処理
}
}
上記のようなクラスがあるとき、クラス定義を抽出すると以下のようになります。
class Register {
userId: string
password: string
execute(): void
}
これだけ見ると情報が欠落していて、どのように使用するクラスか分かりません。
使用するためには実装を開き、クラスの内部を確認する必要があります。
まずはクラス名を変更します。どのような責務を持つクラスなのか、それがわかる具体的な命名をします。抽象的な名前はGODクラスへの第一歩です。
- class Register {
+ class UserCreateCommand {
public userId: string
public password: string
public execute() {
if (!this.userId) { throw Error("userIdに値を入れてください") }
if (!this.password) { throw Error("passwordに値を入れてください") }
// ユーザー作成処理
}
}
クラス名を修正したことで、ユーザーを作成するクラスということが解るようになりました。
これだけだと、まだuserIdとpasswordが必須という情報が読み取れません。
「userIdとpasswordが必須」という情報を明示するというより、コンストラクタを使用してuserIdとpasswordを渡さないとインスタンス化できないよう制限します。
class UserCreateCommand {
- public userId: string
- public password: string
+ constructor(public userId: string, public password: string) { }
public execute() {
if (!this.userId) { throw Error("userIdに値を入れてください") }
if (!this.password) { throw Error("passwordに値を入れてください") }
// ユーザー作成処理
}
}
コンストラクタでパラメーターを受け取るように変更したので、バリデーションもコンストラクタで行うことが望ましいでしょう。
class UserCreateCommand {
constructor(public userId: string, public password: string) {
+ if (!this.userId) { throw Error("userIdに値を入れてください") }
+ if (!this.password) { throw Error("passwordに値を入れてください") }
}
public execute() {
- if (!this.userId) { throw Error("userIdに値を入れてください") }
- if (!this.password) { throw Error("passwordに値を入れてください") }
// ユーザー作成処理
}
}
この段階でも、だいぶ使いやすくなったと思いますが、最後にフィールドにreadonlyの制約をつけます。これによりフィールドがコンストラクタ以外で変更されていないことが保証されます。
class UserCreateCommand {
- constructor(public userId: string, public password: string) {
+ constructor(public readonly userId: string, public readonly password: string) {
if (!this.userId) { throw Error("userIdに値を入れてください") }
if (!this.password) { throw Error("passwordに値を入れてください") }
}
public execute() {
// ユーザー作成処理
}
}
最終的にクラス定義を抽出すると以下のようになります。
class UserCreateCommand {
readonly userId: string
readonly password: string
constructor(userId: string, password: string)
execute(): void
}
どのような使い方をするのか、外から見ただけで理解できるようになりました。
レガシーコードを隠蔽する
レガシーコードやレガシーAPIを扱っていると大量のオプショナルな引数をもつ「何でもデータクラス」のような実装に出会うことがあります。これらは引数の渡し方によって振る舞いを制御します。
以下のLegacyApiは、User型の値の詰め方により、一般ユーザーと法人ユーザーの作成を制御しています。このAPIはカプセル化ができておらず、APIの担当者に使用方法を訪ねに行くことになるでしょう。
type User = {
name: string
mail: string
address?: string // 一般ユーザーの場合のみ必須
corporateUser?: boolean // 法人ユーザーの場合に「true」
corporateAddress?: string // 法人ユーザーの場合のみ必須
}
class UserCreateFacade {
public createUser() {
const api = new LegacyApi()
const user: User = {
name: "yamada taro",
mail: "[email protected]",
address: "神奈川県鎌倉市...",
// パラメーター足りてる…??
}
api.createUser(user)
}
public createCorporateUser() {
const api = new LegacyApi()
const user: User = {
name: "株式会社 good codes",
mail: "[email protected]",
corporateUser: true,
corporateAddress: "東京都品川区...",
// addressも必要…??
}
api.createUser(user)
}
}
こういったAPIは同じチームのメンバーが2度、APIの担当者に使用方法を尋ねることが無いように、パラメーターの生成をFactoryクラスに定義しておくと良いでしょう。
type User = {
name: string
mail: string
address?: string // 一般ユーザーの場合のみ必須
corporateUser?: boolean // 法人ユーザーの場合に「true」
corporateAddress?: string // 法人ユーザーの場合のみ必須
}
class UserFactory {
public static getUser(
name: string,
mail: string,
address: string
): User {
return { name, mail, address }
}
public static getCorporateUser(
name: string,
mail: string,
corporateAddress: string
): User {
return { name, mail, corporateAddress, corporateUser: true }
}
}
class UserCreateFacade {
public createUser() {
const api = new LegacyApi()
const user = UserFactory.getUser(
"yamada taro",
"[email protected]",
"神奈川県鎌倉市..."
)
api.createUser(user)
}
public createCorporateUser() {
const api = new LegacyApi()
const user = UserFactory.getCorporateUser(
"株式会社 good codes",
"[email protected]",
"東京都品川区..."
)
api.createUser(user)
}
}
凝集度と結合度
高凝集
ユーザーの作成とユーザー作成後、操作履歴を登録するサービスクラスを例とします。
type InputData = { userId: string; password: string }
class CreateUserService {
private readonly userApi = new UserApi()
private readonly historyApi = new HistoryApi()
public handle(inputData: Readonly<InputData>) {
this.userApi.create(inputData.userId, inputData.password)
this.historyApi.add(inputData.userId, "new user created.")
}
}
class UserApi {
create(userId: string, password: string) {}
}
class HistoryApi {
add(userId: string, message: string) {}
}
上記のクラスはhandle()で使用するフィールドが全て同クラス内にあり、フィールドとメソッドの関連性が高い状態にあります。このように処理とデータの場所が近いクラスやモジュールを凝集度が高い(高凝集) といいます。
疎結合
現在UserCreateServiceは、UserApiとHistoryApiクラスに依存した実装になっています。それぞれをインターフェースに変更し、クラスへの依存を排除します。
type InputData = { userId: string; password: string }
class CreateUserService {
constructor(
private readonly userApi: UserApi,
private readonly historyApi: HistoryApi
) {}
public handle(inputData: Readonly<InputData>) {
this.userApi.create(inputData.userId, inputData.password)
this.historyApi.add(inputData.userId, "new user created.")
}
}
// classからinterfaceへ変更
interface UserApi {
create(userId: string, password: string): void
}
interface HistoryApi {
add(userId: string, message: string): void
}
このように実装をインターフェースのに依存させたことでUserCreateServiceはどのクラスとも依存関係がない独立したクラスとなりました。
このようなクラスやモジュールを結合度が低い(疎結合) といいます。
高凝集、疎結合まとめ
高凝集、疎結合なクラスは
- 可読性が高い
- 保守性が高い（影響範囲が限定されているため改修が容易）
- 改修箇所のコンフリクトが発生しづらい
- テストコードの作成が容易
など沢山のメリットがあります。
極論ですが、高凝集で疎結合な構成で作られていたら何とかなります。 それ程までに保守性における大切な要素だと思います。
継承は基本的に使わない
継承は適切に使用しないと低凝集・密結合なGODクラスになりやすいです。
基底クラスに共通ロジックを作ることで複雑性が増しているプログラムをよく見かけます。
共通化を図りたいのであれば別のクラスに切り出したほうが、ほとんどの場合で上手くいきます。
public class Base {
public common() {}
}
public class MyClass extends Base {
public method() {
this.common()
}
}
public class Shared {
public common() {}
}
public class MyClass {
public method() {
const shared = new Shared()
shared.common()
}
}
無理に共通化しない
プログラミングを学びたてのときはコードの記述量が少ないことが正義だと思い、過度なDRYを行ってしまうことがありました。
コンテキストが違うものを強引に切り出しても、リリース当初はうまくいきますが、後々、改修のときに苦労します。
結局それぞれのモジュールに分解したり、条件分岐を追加したりと…
新しく作るときより、既存に手を入れるときの方が考えることが多いです。
例えば、人間が「走る」のと動物が「走る」は最初は同じ処理で良いかもしれません。ですが後々、人間にのみ機能を追加する可能性は十分にあります。
処理が共通化されていると人間と動物「両方の走る」を考慮したうえで改修・検証を行う必要がでてきます。
今は「たまたま」処理が同じだけれども、本質的に別の場合は無理に共通化せず、冗長なコードを許容することも大切です。
Dependency Injection（DI）
DIを使いクラス間の依存を排除することには多くのメリットがあります。
ここでは「可読性向上」と「交換可能」という特性をピックアップしたいと思います。
可読性向上
ユーザーを作成する処理を例に取ります。要件は以下になります。
- 開始ログ
- ユーザー作成
- 終了ログ
- ログはテキスト出力とコンソールへの出力とで使い分けることができる。
- テスト・開発・本番でユーザー作成の処理を変更することができる。
DIを使わない場合
class CreateUserHandler {
public handle() {
const logger =
config.logger === "console" ? new ConsoleLogger() : new TextLogger();
logger.info("start create user.");
if (config.env === "test") {
const service = new CreateUserServiceTest();
service.handle();
} else if (config.env === "development") {
const service = new CreateUserServiceDev();
service.handle();
} else {
const service = new CreateUserServiceProd();
service.handle();
}
logger.info("end create user.");
}
}
mm..🤔その他コード
const config = {
env: "production",
logger: "text",
};
class CreateUserServiceTest {
public handle() {}
}
class CreateUserServiceDev {
public handle() {}
}
class CreateUserServiceProd {
public handle() {}
}
class ConsoleLogger {
public info(msg: string) {}
}
class TextLogger {
public info(msg: string) {}
}
ご覧の通り、CreateUserHandlerにロジックとは関係のない条件分岐がノイズとして紛れ込んでおり、処理が追いづらい状態です。
循環的複雑度の考え方では、条件分岐の数だけ複雑度が増すと言われています。
DIを使う場合
class CreateUserHandler {
// 今回は最低限の実装
// ライブラリではデコーレターやコンストラクタを使用してインジェクションすることが多い
private readonly logger = diContainer.get<ILogger>("Logger");
private readonly createUserService = diContainer.get<ICreateUserService>("CreateUserService");
public handle() {
this.logger.info("start create user.");
this.createUserService.handle();
this.logger.info("end create user.");
}
}
good😊その他のコード
const config = {
env: "production",
logger: "text",
};
interface ICreateUserService {
handle();
}
interface ILogger {
info(msg: string);
}
class CreateUserServiceTest implements ICreateUserService {
public handle() {}
}
class CreateUserServiceDev implements ICreateUserService {
public handle() {}
}
class CreateUserServiceProd implements ICreateUserService {
public handle() {}
}
class ConsoleLogger implements ILogger {
public info(msg: string) {}
}
class TextLogger implements ILogger {
public info(msg: string) {}
}
class PoorDiContainer {
private readonly container: { [key: string]: unknown } = {};
public add(key: string, instance: unknown) {
this.container[key] = instance;
}
public get<T>(key: string): T {
return this.container[key] as T;
}
}
// ロジックとは別の所でDIコンテナに依存関係をセットしていく
const diContainer = new PoorDiContainer();
if (config.env === "test") {
diContainer.add("CreateUserService", new CreateUserServiceTest());
} else if (config.env === "development") {
diContainer.add("CreateUserService", new CreateUserServiceDev());
} else {
diContainer.add("CreateUserService", new CreateUserServiceProd());
}
if (config.logger === "console") {
diContainer.add("Logger", new ConsoleLogger());
} else {
diContainer.add("Logger", new TextLogger());
}
configによるクラスの振り分けをdiContainerへの登録時に処理することで、CreateUserHandlerはシンプルな実装になりました。
interfaceの定義などで「その他のコード」は増えましたが、カプセル化が適切に行われている場合、確認するところはinterface部分だけなので「その他のコード」の記述量は気になりません。
開発のフェーズに合わせて都合の良い実装に差し替える
テストコードの実行では副作用のある処理を入れたくありません。
この場合もCreateUserHandlerにテスト用の記述をする必要なく、以下のようにconfigの変更だけでテスト用の実装に変更することができます。
const config = {
env: "test", // テスト用に副作用のない実装に切り替える
logger: "console", // テスト用に副作用のない実装に切り替える
};
describe("ユーザー作成", () => {
it("正常終了", () => {
const createUserHandler = new CreateUserHandler();
createUserHandler.handle();
});
});
もし、新たなLoggerを追加するときも、変更箇所はDIコンテナに追加するところだけです。
// DIコンテナから参照されている設定とする
const config = {
env: "production",
logger: "mongodb",
}
if (config.logger === "mongodb") { // new!!
diContainer.add("Logger", new MongoDbLogger())
} else if (config.logger === "console") {
diContainer.add("Logger", new ConsoleLogger())
} else {
diContainer.add("Logger", new TextLogger())
}
このようにDIを使用するとクラス間の依存関係が薄くなり、独立性の高いモジュール群が作られ、それぞれの開発フェーズにあった実装を容易に切り替えることが可能になります。
実運用ではDIコンテナと呼ばれる種類のライブラリを使い、依存関係を管理すると良いでしょう。
インスタンスをキャッシュしたり、ファクトリーパターンによるインスタンスの生成など、DIをより快適に使うための機能が付属しています。
Dependency Inversion Principle(DIP)
SOLID原則の1つで和訳では「依存性逆転の原則」と言われています。
DIPを使用した有名な設計にリポジトリパターンがあります。
上位モジュールをインターフェースに依存させ、DIにより下位モジュールを切り替えます。それにより、永続化に関する処理をビジネスロジックから切り離すことができ、データに関する処理が抽象化されます。
このように依存関係を逆にすることで、本当に大事なドメイン部分の実装を下位モジュールの都合で変更する必要がなくなります。
ドメイン部分をシステムの中心に据え、変更の多いレイヤーを外側に持っていく考え方は、オニオンアーキテクチャやクリーンアーキテクチャでも採用されています。
ジェネリクスを使う
ジェネリクスは慣れるとなんてことはないですが、初学者にとっては新しい概念であり、はじめのころは理解するのに苦しむと思います。
また、使用することは感覚的にできても、ジェネリクスを使ったモジュールを作成し、設計に組み込むにはまた違う難しさがあると思います。
それゆえ、静的型付け言語における初学者の登竜門という位置付けで私は考えています。
ジェネリクスは言語によってはボックス化の回避やキャストによるランタイムエラーの抑制、インターフェースの表現力強化などに利用でき、とても便利です。
また、扱う型を呼び出し元から指定できるので、処理に一貫性を持たせつつ、呼び出し元に都合の良いモジュールを作成することができます。
型指定のないリスト
class AnyList {
private readonly source: any[] = []
public add(value: any) {
this.source.push(value)
}
public get(index: number): any {
return this.source[index]
}
}
const anyList = new AnyList()
anyList.add("xxx")
const value = anyList.get(0) // この時点ではany型
if (typeof value === "string") {
// ここではstring型として扱われる
}
TypeScriptを例に取るとタイプガードにによって型を絞り込むまで、any型として扱われます。
ゆえに型を特定したうえで使用する必要があります。
また、万が一違う型にキャストしてしまった場合はランタイムエラーが発生してしまうので、常に気を配っておく必要があります。
Genericsを使用したリスト
class GenericsList<T> {
private readonly source: T[] = []
public add(value: T) {
this.source.push(value)
}
public get(index: number): T {
return this.source[index]
}
}
const genericsList = new GenericsList<string>()
genericsList.add("xxx")
const value: string = genericsList.get(0) // この時点でstring型
ジェネリクスを使用し、型定義を呼び出し元から指定することでstring型でもnumber型でもobject型でも、どの型にも対応できるクラスを作成することができました。
アーキテクチャーを参考にする
アーキテクチャーは先人たちの偉大な発明です。これを参考にしない手はないです。
ここでは以下を取り上げます。詳細は専用の記事がたくさんあるのでそちらをご覧ください。
- クリーンアーキテクチャー、オニオンアーキテクチャー
- CQSとCQRS
クリーンアーキテクチャー、オニオンアーキテクチャー
この2つに共通している最も重要な考えは「依存関係を円の中心に向ける」ことだと私は認識しています。
それはリポジトリパターンなどを使用し、依存関係を逆転することで実現します。
依存の方向を外から内にのみ許可すること。ビジネスロジックを中心にすること。これを遵守するこでその他のレイヤーへのビジネスロジックの漏洩を防ぎ、ドメイン部分が独立します。
最も大切なビジネスロジックが独立していると、その他のレイヤーは交換可能（プラガブル）なパーツとなり、全体がテストの容易なメンテナンス性の高いプログラムとなります。
こちらの記事がとても参考になります。
CQSとCQRS
これらは、データの取得（Query）とデータの変更（Command）では関心ごとが違うので、単純なCRUDよりもQueryとCommandを分離したほうがいろいろうまくいくよね、という考えから来ています。
プログラムの内部でQCを分離しているものをCQSと言い、サーバーやリソース単位で分離しているものをCQRSと言います。
サーバー単位で分離しているものでは、イベントソーシングがよく一緒に使われます。
イベントソーシングはユースケースをイベントとしてとらえ、DBにイベントを積み上げていきます。Deleteにあたるユースケースも削除のイベントとしてDBにインサートします。（gitのイメージ）
積み上げられたイベントはイベントジャーナルと呼ばれ、Commandからのみインサートされるデータソースとなります。
逆にQueryではイベントジャーナルから抽出した都合の良いデータでリードモデルの役割を持つDBを作成します。
都合の良いデータ構造は取得時のパフォーマンスが高く、表現力が豊かです。
リードモデルはトランザクションログを監視したり、DBに備わっている変更のHook（※1）でデータソースの変更を検知し、更新をかけることで最新の状態を維持します。
※1 DynamoDB Stream やOracleの CHANGE NOTIFICATION のことです。
自動生成に頼る
人間が作成すると、その人の思想がコードに入ります。ときにそれは可読性を下げたり混乱を生じる事もあります。
例えばAPIのラッパーやDBのスキーマに対応したEntityなどは機械的に作られている方が、利用する側としては使いやすいです。
DBスキーマのスキャフォールディングは歴史が長いですが、APIに関してもOpenAPIやGraphQL、gRPCなどの仕様に乗ることでAPIリクエストの一連のコードを自動生成することができます。
私はOpenAPIでのコード生成を使用することが多いのでその例を記します。業務ではバックエンドAPIが.NETで、フロントエンドがReact.jsまたはVue.jsを使用することが多いです。
この場合もバックエンドで作成したOpenAPI定義ファイルのYAMLから、各エンドポイントへのリクエストの処理をラップしたSDKを openapi-generator で自動生成しています。
エンドポイントの定義が変わったとしても生成し直すだけなので、メンテナンスコストがかからず、かつC#とTypeScript、言語は違えどOpenAPI定義ファイルを通してタイプセーフな実装を行うことができています。
また、会社のシステムでは上述したバックエンドAPIの更に下位にもレイヤーがあり、それらもAPIでつながっています。それらのAPIもOpenAPI定義ファイルを公開しているため、使用する側は定義ファイルからSDKを生成しています。
こちらの記事でバックエンド.NETとフロントエンドTypeScriptを、OpenAPI定義ファイルを通してSDKを自動生成し、タイプセーフを実現する方法を記しているので興味ある方はぜひ御覧ください。
まとめ
以上が私が日頃意識している「良いコード」を書くためのTipsでした。
私の環境ではこれらの内容がとてもよく当てはまります。ですが皆さんの環境ではどうでしょうか？
昨今はOOPや設計に関する参考書がたくさん出版されています。ですが、自分の業務環境にその本の内容をマージしようとしても、うまくいかないことはとても多いです。
それは本の作者がサンプルで作っているシステムと、あなたのシステムとでは求められていることも、環境もまるで違うからです。
結局、良いコードを書くためには、そのための引き出しをたくさん作り、自分の環境にどのように取り入れたらコンフリクトが起きないか、トライアンドエラーを繰り返し、勉強と経験を積み上げていくしかないと思っています。
私も日々、次回は今よりも良いコードを書きたいと思い、インプットを続けています。
また何か良いTipsを言語化できたとき、この記事を更新しようと思います。
長文&最後ポエムになってしまいました。笑
読んでくださった方、貴重なお時間を割いていただき、ありがとうございました🙌
CHANGELOG
- 2022-12-18
- 「#コメントは背景から書く」の条件分岐を反転（頂いたコメントから早期リターンだと分かりづらいと判断しました）
- フィールド名「id」を「userId」にリファクタリング
- メソッド名「validateXxx」を「isXxxValid」にリファクタリング
- classの構文エラーを修正
Discussion
早期リターンの改善後のコードが以下になってますが、if文の判定に
!が足りていないように思います。ほんとですね！
教えてくれてありがとうございます。取り急ぎ修正しました。
今思うと「validateXX」は曖昧な名前でしたね。（しっかり自分で引っかかってしまいました😅）
DeepReadonlyを定義して、ReadonlyにDeepReadonlyしたり、
eslintとprettierで自動フォーマットするとよさそう🙆♂
DeepReadonly初めて聞きました！（ありがとうございます!!）
調べてみたら下記のリポジトリが便利そうなので、今度眺めてみようと思います👀
ですね！実際にTypeScriptで運用するときはどちらも組み込むようにしています🙆♂️
突然ですみません、第一目のコードなんですが、ここ間違っているじゃないでしょうか？
ここ
||ではなく、&&なのでは？日本語は下手なので、誤解したらすみません。
ここ、、サンプルコードが良くなかったですね。
早期リターンを例にせず、以下のようにしたほうが伝わりやすかったです。
早期リターンにする場合だと以下だけで条件クリアしているので
isOldTypeAccountの判定は不要ですものね。ご指摘ありがとうございます！
時間あるときに修正したいと思います🙆♂️