前置き
私が所属しているチームの輪読会で『良いコード/悪いコードで学ぶ設計入門』が選ばれました。
メンバーがそれぞれ章を分担して毎週発表をするスタイルになります。
書籍に書いてあることではなく、自身の言葉で説明要約するために、個人のアウトプットとして本記事を作成しました。
本記事で利用しているコードは、原則的には書籍の内容とは別物 + TypeScriptを利用しています。
12章 良きクラスには良きメソッドあり
低凝集になる設計
カプセル化で調べるとgetter, setterがよく出てくると思いますが、外部から値を自由に出し入れできる構造は低凝集なクラスになりがちです。
ユーザーが入力したシミュレーション値の状態管理を行うクラスがあるとして、このクラスの問題点を修正していきたいと思います。
type State = Record<string, any> & {
totalLoan: Amount,
};
// 小数点を含まない正の整数として別クラスのコンストラクタで判定すること
type Amount = { value: number };
type Response = { status: number };
class SimulationStore {
state: State;
constructor(state: State) {
this.state = state;
}
// 更新されたデータがあるか確認(仮処理)
hasUpdatedData(): boolean {
return Object.keys(this.state).length > 0;
}
// DBにデータを更新する(仮処理)
async updateToDB(): Promise<Response> {
const response: Response = await new Promise((resolve) => {
resolve({ status: 200 });
});
return response;
}
}
インスタンス変数を外部から変更できる構造にしてはいけない
ローン計算を行うクラスで結果をシミュレーションを保存までしていますが、このような使い方にはいくつかの問題があります。
- クラス同士が密結合になっている
- 入力値であるはずの引数が、メソッド内で値を変えられて出力引数となっている
これらは「してはいけない」と言うよりかは「構造上できてはいけない」設計にするのが望ましいです。
class LoanCalculator {
calc(store: SimulationStore): void {
/* 計算処理の算出結果を200と仮定 */
const totalLoan = new Amount(200);
store.state.totalLoan = totalLoan;
}
}
LoanCalculatorクラスでは計算結果を返すまでとし、保存処理についてはSimulationStoreクラスで行うようにしましょう。
class LoanCalculator {
- calc(store: SimulationStore): void {
+ calc(store: SimulationStore): Amount {
/* 計算処理の算出結果を200と仮定 */
const totalLoan = new Amount(200);
- store.state.totalLoan = totalLoan;
+ return totalLoan;
}
}
保存項目はprivateとし更新用のmutateメソッドを実装します。
class SimulationStore {
- state: State;
+ private state: State;
/* 中略 */
+ public mutateTotalLoan(totalLoan: Amount):void {
+ this.state.totalLoan = totalLoan;
+ }
}
インスタンス変数を不変にする
初期化したSimulationStoreクラスが、複数の箇所でユーザー操作やシステムによって値が更新/参照されるとします。
タイミングや別プロセス(並行処理)によっては上書きし合うため、意図しない値が保存/計算される可能性があります。
下の例で言えば、Aのボタン操作時には100だった値が「非同期な重い処理」を行っている最中に、「リセット処理」が入ると0に上書きされてしまいます。
// ユーザーがAの操作をした時の処理
async function execButtonA(input: number): Promise<void> {
const totalLoan = new Amount(input);
store.mutateTotalLoan(totalLoan);
// 非同期な重い処理
await heavyAsync(store);
}
// リセット処理
function resetTotalLoan(): void {
const totalLoan = new Amount(0);
store.mutateTotalLoan(totalLoan);
}
インスタンス変数をreadonlyとし、mutateメソッドで新しくインスタンスを生成して返す手法を取ることで、不変にしつつも更新されたデータを参照することができるようになります。
※ readonlyはJavaであればfinal修飾子と同義です
インスタンス生成を繰り返すことによって、メモリを大幅に消費したり操作性が低下する可能性があります。必ずしもこの構成を利用する必要はありませんが、並行処理によって意図しない値が入る可能性を想定して設計をする必要はあります。
- type State = Record<string, any> & {
+ type State = Readonly<Record<string, any> & {
totalLoan: Amount,
- };
+ }>;
/* 中略 */
class SimulationStore {
- private state: State;
+ private readonly state: State;
/* 中略 */
- public mutateTotalLoan(totalLoan: Amount):void {
- this.state.totalLoan = totalLoan;
+ public mutateTotalLoan(totalLoan: Amount): SimulationStore {
+ const state = {
+ ...this.state,
+ totalLoan,
+ };
+ return new SimulationStore(state);
}
/* 中略 */
}
尋ねるな、命じろ
デメテルの法則にある文言なのですが、以下のようにクラス外で 「更新データはある?」 「持ってるんだったらDBに保存して」 というような処理は違反している状態と言えます。
const store = new SimulationStore({});
/* 計算処理など中略 */
if (store.hasUpdatedData()) {
await store.updateToDB();
}
class SimulationStore {
/* 中略 */
// 更新されたデータがあるか確認(仮処理)
hasUpdatedData(): boolean {
return Object.keys(this.state).length > 0;
}
// DBにデータを更新する(仮処理)
async updateToDB(): Promise<Response> {
const response: Response = await new Promise((resolve) => {
resolve({ status: 200 });
});
return response;
}
}
「尋ねるな、命じろ」というのは、内部の状態を呼び出し側が判断せずに命令された側で判断や制御をするように設計しましょう、ということです。
必要な手続きのみを公開(public)して、データに関する処理をまとめた高凝集なクラスにしましょう。
class SimulationStore {
/* 中略 */
+ public async tryUpdateToDB(): Promise<void> {
+ if (this.hasUpdatedData()) {
+ await this.updateToDB();
+ }
+ }
// 更新されたデータがあるか確認(仮処理)
- hasUpdatedData(): boolean {
+ private hasUpdatedData(): boolean {
return Object.keys(this.state).length > 0;
}
// DBにデータを更新する(仮処理)
- async updateToDB(): Promise<Response> {
+ private async updateToDB(): Promise<Response> {
const response: Response = await new Promise((resolve) => {
resolve({ status: 200 });
});
return response;
}
}
コマンド・クエリ分離
メソッドはコマンド(データ/状態の変更)とクエリ(データ/状態の取得)のどちらか一方だけを行うよう設計する考え方があります。
以下はモディファイア(変更と取得)型で計算処理とその結果を返しています。
class SimulationStore {
/* 中略 */
public addIncome(income: Amount): Amount {
this.state.income += income;
return this.state.income;
}
}
収入を加えるaddIncomeメソッドの返り値が、計算後の値となっているのは利用者が混乱する上、更新だけ/取得だけしたい場合に対応できません。
そのためコマンドとクエリにメソッドを分離することとします。
class SimulationStore {
/* 中略 */
- public addIncome(income: Amount): Amount {
+ public addIncome(income: Amount): void {
this.state.income += income;
- return this.state.income;
}
+ public getIncome(): Amount {
+ return this.state.income;
+ }
}
本記事の「インスタンス変数を不変にする」で扱ったように新しいインスタンスを結果として返す手法や、メソッドチェーン・流れるようなインターフェースなどで既存インスタンス自身を返す手法もあります。特殊な手法を用いる場合を除いて基本的には分離するように心がけましょう。
nullを渡さない、返さない
nullが渡されることを前提としたロジックではnullチェックが必須で、チェックが行われない場合はIDEやコンパイルでエラーとなる可能性があります。
Invalid type "string | null" of template literal expression.
以下はnullableなデータベースからデータを取得した後に、各所でnullチェックする実装です。
addressを文字列として利用するあらゆる場面でチェックが必要になります。最近はIDEで指摘されるので、nullチェック漏れが発生することはないと思いますが、各所でnullチェックによるメソッドの複雑化を招くことになります。
type UserProfile = {
name: string,
address: string | null,
};
async function fetchUserProfile(): Promise<UserProfile> {
const userProfile: UserProfile = await new Promise((resolve) => {
resolve({ name: 'Foo Bar', address: '東京' });
});
return userProfile;
}
async function consoleLivesIn(): Promise<void> {
const userProfile = await fetchUserProfile();
const { name, address } = userProfile;
if (address !== null) {
console.log(`${name}さんは${address}に住んでいます`);
// Foo Barさんは東京に住んでいます
}
}
データベースやライブラリなどnullが混じるデータはいくらでもあるので、全てに対応するのは難しいかと思いますが、以下のようなnullチェックをクラス内で行う実装をしても良いかもしれません。
理想としてはnullのないコードにしたいとこですが、IDEでサポートを受けつつ速度重視で開発することが多そうではあります。
type UserProfile = {
name: string,
address: string | null,
};
// ※ プロパティの型を上書きをする良い方法があればそちらに変えてください
+ type NotNullUserProfile = Omit<UserProfile, 'address'> & {
+ address: Address,
+ };
+ class Address {
+ public readonly value: string;
+ public readonly isNullOrigin: boolean;
+ constructor(address: string | null) {
+ this.value = address ?? '【住所不定】';
+ this.isNullOrigin = address === null;
+ }
+ }
- async function fetchUserProfile(): Promise<UserProfile> {
+ async function fetchUserProfile(): Promise<NotNullUserProfile> {
const userProfile: UserProfile = await new Promise((resolve) => {
resolve({ name: 'Foo Bar', address: '東京' });
});
- return userProfile;
+ const address = new Address(userProfile.address);
+ return { ...userProfile, address };
}
async function consoleLivesIn(): Promise<void> {
const userProfile = await fetchUserProfile();
const { name, address } = userProfile;
- if (address !== null) {
- console.log(`${name}さんは${address}に住んでいます`);
- }
+ console.log(`${name}さんは${address.value}に住んでいます`);
// Foo Barさんは東京に住んでいます
}
引数は不変にする
オブジェクト型を引数に渡す場合は参照渡しとなっているため、引数を予期せぬ値で上書きすることが可能です。
取得したレコードを加工するためにメソッドに渡す想定の例を出します。
type Response = {
firstName: string,
lastName: string,
age: number,
};
type TableRecord = {
fullName: string,
age: number,
};
function convertToRecord(response: Response): TableRecord {
const { firstName, lastName, age } = response;
// 引数が不変でないため、引数responseの元々の値も変わる
response.age += 10;
const fullName = `${lastName} ${firstName}`;
return { fullName, age };
}
const response: Response = {
firstName: 'Foo',
lastName: 'Bar',
age: 26,
};
const tableRecord: TableRecord = convertToRecord(response);
console.log(response);
// { firstName: 'Foo', lastName: 'Bar', age: 36 }
console.log(tableRecord);
// { fullName: 'Bar Foo', age: 26 }
Readonlyを付与することで不変にすることができます。以下のように変更すればIDEとコンパイルでエラーになります。
※ Javaで言うところのfinalと同じ使い方です。
極端な話ですが、「Readonlyを付けてない = 上書きをする設計であり上書きしても問題ない」と捉えることもできます。JavaScriptにおけるconstとletの関係に近いものがあります。
/* 中略 */
- function convertToRecord(response: Response): TableRecord {
+ function convertToRecord(response: Readonly<Response>): TableRecord {
const { firstName, lastName, age } = response;
// Cannot assign to 'age' because it is a read-only property.
response.age += 10;
const fullName = `${lastName} ${firstName}`;
return { fullName, age };
}
/* 中略 */
参照渡しのオブジェクトでなくプリミティブな値であれば、引数を上書きしてもいいのかと言えばこれもNGです。
下記例では3行のコードですが、数十行に渡る条件分岐を含んだ処理で引数valueを上書きし続けると、コードの中でどの値が入っているかを追うのが難しくなります。
function calc(value: number): number {
value *= 1.1; // 消費税
value += 500; // 配送料
return value;
}
console.log(calc(1000));
// 1600
消費税率や配送料は変数として定義し、calcメソッド内での計算結果をローカル変数として命名するように変更しました。こうすることで引数を不変にしつつ、リーダブルなメソッドとなります。
+ const TAX_RATE = 1.1;
+ const DELIVERY_CHARGE = 500;
- function calc(value: number): number {
- value *= 1.1; // 消費税
- value += 500; // 配送料
- return value;
+ function calc(price: number): number {
+ const taxPrice = price * TAX_RATE;
+ const totalPrice = taxPrice + DELIVERY_CHARGE;
+ return totalPrice;
}
console.log(calc(1000));
// 1600
TypeScriptではプリミティブな引数にReadonlyを使うことはできません。あくまでもオブジェクト内のプロパティにのみ適応されます。
紹介はしませんがreadonly修飾子を利用することで、クラス変数や配列の中身の変更を禁止することもできます。
※ 型に使うReadonlyと修飾子のreadonly は別物です
フラグ引数を作らない
免税やテイクアウトで基本税率と異なる計算をしたい場合に、フラグを用いて計算処理をする例です。
このcalc()メソッドを利用するエンジニアが謎のboolean型第2引数を見て、税率の違いを判定するとは初見で思い至らないでしょう。
コメントなど残っているかと思いますが、実装の中身を見てやっと第2引数の意味を理解することになります。こういった個別の実装を見ないと把握できないメソッドがあると可読性と生産性が低下します。
詳細は省きますが、こういった条件分岐はストラテジパターンを用いた税関係の計算Interfaceで処理するのが良いとされています。
const TAX_RATE = 1.1;
const DELIVERY_CHARGE = 500;
function calc(price: number, flag: boolean): number {
const taxPrice = price * (flag ? TAX_RATE : 1);
const totalPrice = taxPrice + DELIVERY_CHARGE;
return totalPrice;
}
console.log(calc(1000, false));
// 1500
引数は可能な限り少なくする
引数が多いということは、それだけ処理内容が多くてロジックが複雑になりやすいです。また同型の引数を複数渡すようなメソッドの場合、その順番を間違える可能性もあります。
以下例では複雑な処理は行いませんが、同型の引数を順番に渡す必要のあるメソッドです。
修正例はありませんが、別のクラスやメソッドに分割して計算するなどして、引数は2〜3個に抑えるようにしましょう。Linterでは引数の数で警告を出す設定もあるので活用してもいいかもしれません。
function calc(
originPrice: number,
discountRate: number,
taxRate: number,
deliveryCharge: number,
) {
const discountPrice = originPrice * (1 - discountRate);
const taxPrice = discountPrice * (1 + taxRate);
const totalPrice = taxPrice + deliveryCharge;
return totalPrice;
}
const totalPrice = calc(1000, 0.3, 0.1, 500);
console.log(totalPrice);
// 1270
引数と戻り値に型を使う
プリミティブ型をそのまま利用するのではなく、例のような意味のある型にすることが推奨されます。
特にJavaScript, TypeScriptでは数字にintやfloatといった括りがなくnumber型しかありませんので、オリジナルの型(クラス)を作成することで不正な値が入らないようにして、後の計算処理を正しく行うことが可能になります。
class Price {
public readonly value: number;
constructor(value: number) {
if (!Number.isInteger(value)) throw new Error(`${value}は整数ではありません`);
if (value < 0) throw new Error(`${value}は正の整ではありません`);
this.value = value;
}
}
class Rate {
public readonly value: number;
constructor(value: number) {
if (Number.isInteger(value)) throw new Error(`${value}は整数です`);
if (value < 0) throw new Error(`${value}は正の整ではありません`);
this.value = value;
}
}
数字だけでなく文字列も厳密に指定することが可能です。
もし固定の文字列しか扱わない変数やプロパティがあれば、文字列リテラルのユニオン型にして明示的にするのも手です。
※ 他言語における列挙型に近いものです
const rankTypes = ['ブロンズ', 'シルバー', 'ゴールド'] as const;
type RankType = (typeof rankTypes)[number];
// 型 '"マスター"' を型 '"ブロンズ" | "シルバー" | "ゴールド"' に割り当てることはできません。
const rank: RankType = 'マスター';
まとめ
書籍の中では総集編に近い内容で、これまでの内容がまとめられていた章でした。説明仕切れない部分も中にはあります。
調べてTypeScriptで自分風に書き直して実行して…としていたので、かなり時間がかかりました。とはいえ、自分でコードを書くことで「良いコードと悪いコード」の例をより理解できたため、無駄ではないと感じています。
読み物として見にくければ、記事を2分割にするか節ごとに分けた方が良さそうな気はしていますが、一旦はこのままで。