良いコードの定義とは? 責務の分離で保守性を高める
コードを書く時の注意点っていろいろあると思うのですが、自分の中で、大事にしている観点の1つはなるべく短くかく です。
短くかく理由ですが、単一責任の原則により、一つのメソッド、クラスの責務が分離されていれば自然と短くなっていくと思うからです。
本記事ではてTypeScriptのコード例を交えてコードでは１つのことをさせる、なるべく短くかく、ということを実例を挙げて書いていきたいと思います。
単一責任の原則
単一責任の原則を守ることにより、メリットとしては以下のようなことが挙げられます。
- 理解しやすい・読みやすい
1つのことだけをやっているコードは、頭の中で追いかけやすいです。 - 変更が容易・安全
変更の影響範囲が明確で小さいため、自信を持って修正できます。 - テストが簡単
テストしたい部分だけを独立してテストできるようになります。 - 再利用できる
同じ処理を別の場所でも使えるようになります。
単一責任の原則をまもろうとすれば必然的にコードは短くなっていくことが一般的だと思います。
プログラムは長ければ長いほど理解しにくくなります。処理ごとに分割して短いコードを目指しましょう。
具体的な目安
- 基本的には20行以内を目標にする(あくまで目安なので、実情はプロジェクトにより違うと思いますが、１つのことをしているか、という観点ですね。)
- 長くしていいのは、コントローラー的なメソッドか並列的な処理(switch文など)が続く場合のみ
ただし、短ければ良いというわけではありません。無理やり1行にまとめてかえってわかりにくくなっているコードも少なくありません。短くするのはあくまで理解しやすくするための手段です。長く書いたほうが理解しやすい場合は、長く書くべきです。
悪い例: 複数の責務が混在している
// ❌ 悪い例: ユーザー検証、データ取得、整形、保存が1つの関数に
function processUser(userId: string) {
// ユーザーの検証
if (!userId || userId.length === 0) {
throw new Error('Invalid user ID');
}
// データベースからユーザー情報を取得
const user = db.users.findById(userId);
if (!user) {
throw new Error('User not found');
}
// データの整形
const fullName = `${user.firstName} ${user.lastName}`.trim();
const age = calculateAge(user.birthDate);
// 統計情報の更新
db.stats.increment('totalUsers');
// ログ出力
logger.info(`Processed user: ${fullName}`);
// 結果の返却
return { fullName, age, email: user.email };
}
この関数は以下の複数のタスクを行っています:
- 入力の検証
- データの取得
- データの整形
- 統計情報の更新
- ログ出力
良い例: 責務を分離
// ✅ 良い例: 各関数が1つの責務のみを持つ
// 1. 検証
function validateUserId(userId: string): void {
if (!userId || userId.length === 0) {
throw new Error('Invalid user ID');
}
}
// 2. データ取得
function fetchUser(userId: string): User {
const user = db.users.findById(userId);
if (!user) {
throw new Error('User not found');
}
return user;
}
// 3. データ整形
function formatUserData(user: User) {
return {
fullName: `${user.firstName} ${user.lastName}`.trim(),
age: calculateAge(user.birthDate),
email: user.email
};
}
// 4. 統合処理(コントローラー的な役割)
function processUser(userId: string) {
validateUserId(userId);
const user = fetchUser(userId);
return formatUserData(user);
}
各関数が1つの明確な責務を持つことで、テストしやすく、再利用しやすく、理解しやすいコードになります。
もう1つの例: 配列処理
// ❌ 悪い例: 1つの関数で複数の処理
function processOrders(orders: Order[]) {
const result = [];
let total = 0;
for (const order of orders) {
if (order.status === 'completed' && order.amount > 0) {
const tax = order.amount * 0.1;
const finalAmount = order.amount + tax;
total += finalAmount;
result.push({
id: order.id,
amount: finalAmount,
date: formatDate(order.date)
});
}
}
sendEmail(`Total: ${total}`);
return result;
}
// ✅ 良い例: 責務を分離
function filterCompletedOrders(orders: Order[]) {
return orders.filter(o => o.status === 'completed' && o.amount > 0);
}
function calculateOrderTotal(order: Order) {
const tax = order.amount * 0.1;
return order.amount + tax;
}
function formatOrder(order: Order) {
return {
id: order.id,
amount: calculateOrderTotal(order),
date: formatDate(order.date)
};
}
function processOrders(orders: Order[]) {
const completedOrders = filterCompletedOrders(orders);
return completedOrders.map(formatOrder);
}
クラスもなるべく短くかく
上記はメソッドを中心に書きましたが、classでも１つの対象をあつかっていれば必然的に短くなり、
長くとも150行程度になっていくかと思います。
クラスが大きくなりすぎると、理解しにくく、テストしにくく、変更が困難になります。
悪い例: 巨大なクラス(God Object)
// ❌ 悪い例: あらゆる処理を1つのクラスに詰め込んでいる
class UserManager {
// データベース接続
private db: Database;
// ユーザーのCRUD操作
async createUser(data: UserData) {
// バリデーション
if (!data.email.includes('@')) {
throw new Error('Invalid email');
}
if (data.password.length < 8) {
throw new Error('Password too short');
}
// パスワードのハッシュ化
const hashedPassword = await bcrypt.hash(data.password, 10);
// データベースに保存
const user = await this.db.users.create({
...data,
password: hashedPassword
});
// ウェルカムメールの送信
await this.sendWelcomeEmail(user.email, user.name);
// 統計情報の更新
await this.updateStatistics('user_created');
// ログ記録
this.logUserAction(user.id, 'created');
return user;
}
async updateUser(userId: string, data: Partial) {
// 更新処理...
}
async deleteUser(userId: string) {
// 削除処理...
}
// メール送信
private async sendWelcomeEmail(email: string, name: string) {
// メール送信ロジック...
}
private async sendPasswordResetEmail(email: string, token: string) {
// メール送信ロジック...
}
// 統計情報
private async updateStatistics(action: string) {
// 統計更新ロジック...
}
private async getStatistics() {
// 統計取得ロジック...
}
// ログ記録
private logUserAction(userId: string, action: string) {
// ログ記録ロジック...
}
// その他多数のメソッド...
}
このクラスは以下のような問題を抱えています:
- データベース操作、バリデーション、メール送信、統計、ログ記録など、多くの責務を持っている
- 150行以上になり、理解しにくい
- テストが困難(モックすべき依存関係が多すぎる)
- 変更の影響範囲が大きい
良い例: 責務ごとにクラスを分離
// ✅ 良い例: 各クラスが1つの責務を持つ
// 1. バリデーション専用
class UserValidator {
validate(data: UserData): void {
if (!data.email.includes('@')) {
throw new Error('Invalid email');
}
if (data.password.length < 8) {
throw new Error('Password too short');
}
}
}
// 2. パスワード処理専用
class PasswordService {
async hash(password: string): Promise {
return bcrypt.hash(password, 10);
}
async verify(password: string, hash: string): Promise {
return bcrypt.compare(password, hash);
}
}
// 3. メール送信専用
class EmailService {
async sendWelcomeEmail(email: string, name: string): Promise {
// ウェルカムメール送信ロジック
}
async sendPasswordResetEmail(email: string, token: string): Promise {
// パスワードリセットメール送信ロジック
}
}
// 4. ユーザーデータの永続化専用
class UserRepository {
constructor(private db: Database) {}
async create(data: UserData): Promise {
return this.db.users.create(data);
}
async findById(id: string): Promise {
return this.db.users.findById(id);
}
async update(id: string, data: Partial): Promise {
return this.db.users.update(id, data);
}
}
// 5. ユーザー作成のビジネスロジック(各サービスを組み合わせる)
class UserService {
constructor(
private validator: UserValidator,
private passwordService: PasswordService,
private repository: UserRepository,
private emailService: EmailService
) {}
async createUser(data: UserData): Promise {
this.validator.validate(data);
const hashedPassword = await this.passwordService.hash(data.password);
const user = await this.repository.create({
...data,
password: hashedPassword
});
await this.emailService.sendWelcomeEmail(user.email, user.name);
return user;
}
}
分離後の利点としては以下のようなことが挙げられます。
- 各クラスが短くコンパクト
- 1つのクラスが1つの責務のみを持つ
- テストが容易(必要な部分だけモック可能)
- 再利用しやすい(例:
PasswordServiceは他の機能でも使える) - 変更の影響範囲が小さい