読みやすいコードを書くためのガイドライン

はじめに

長くシステム開発に携わっていると、最初は素早く開発することができたけども、今では新たな機能を付け加えるのに、多くの時間を費やすようになったという話をよく聞きます。
初めはシンプルだったソースコードも年月が過ぎていくなかで、機能追加やバグ修正、変更を行なっていくと、コードが非常に複雑化していき、開発者が修正するコストの増加やコード品質の低下につながります。設計の良くないコードや複雑化したコードは、同じ処理をするのにも余計にコードを書く必要がでてくるからです。

今回は、コードを複雑化して品質を低下させないために、普段意識しているソフトウェア設計や手法を紹介していきたいと思います。色々書いていますが、結果的にクリーンアーキテクチャー成分が多めになってしまいました。

「読みやすいコード」が持つメリット

コードを読みやすくすることは開発全体に好影響を与え、特に以下の点において最大限効果を発揮してくれます。

バグの発見を助ける

コードが理解しやすいということは、バグの見つけやすさにつながります。不具合が起きた時に、どこに問題があるかをすぐに特定でできない場合、効率を下げる大きな要因となります。

エラーの発生率の低下

読みやすいコードは、コードの構造が明確であり、変数や関数の命名も適切であるため、誤解や間違いを減らし、コードの品質が向上します。また、開発者がコードを理解しやすいため、新たなバグの導入が減ることになります。

プログラミングの速度を早める

コードの品質が上がり、内部設計やコードの意図が理解されやすくなるなると、新たに機能を加える際にどのように変更すれば良いかがすぐに把握でき、モジュール化されていれば、変更しなければならない箇所を小さく限定することができます。これは開発スピードに絶大なる影響を与えます。

チームで開発するということ

ソフトウェアを開発する場合、多くの場面においてチームで作業をします。大抵は、他のエンジニアが書いたコードの上で開発していくことが多く、また、他のエンジニアも同様に、自分が書いたコードの上に開発していきます。機能は常に変化し、多くのエンジニアが継続的にソースコードに変更を加えるため、堅牢で使いやすいコードでなかった場合、脆弱で壊れやすいものになります。自分にとって明確だと思われるコードも、他の人にとっては明確ではないことがあり、いつの間にかコードが壊されてしまっている可能性があります。そのことを常に意識して、他のエンジニアが書いたコードに関わる際に起こり得る出来事を理解し、先手を打って回避できるように、分かりやすく壊れにくいコードを記載していく必要があります。

TypeDocの活用

TypeScript用のドキュメント生成ツールです。メソット等の処理を補完してくれるので処理の流れが色々とわかりやすくなります。

https://typedoc.org/guides/overview/

/**
 * @param a - the first number
 * @param b - the second number
 */
export function sum(a: number, b: number) {
    return a + b;
}

類似したものでJavaScriptファイルの型情報を提供するJSDocもありますが、型情報を{String}といった表記で記載する必要がないので、個人的にはTypeDocの方が好みです。

簡潔ではあるけど理解できないコードは避ける

コードの行数を減らして、最小限に抑えることで逆に読みにくくなることがあります。

return order.quantity * order.itemPrice −
Math.max(0, order.quantity − 500) * order.itemPrice * 0.05 + Math.min(order.quantity * order.itemPrice * 0.1, 100);

分かりにくいコードが１行あった場合、たとえ同じ場所に分かりやすいコードが20行あったとしても、他のエンジニアはこの1行のコードのために詳細を理解して、必要な情報を取り出すために多くの時間をかける必要があります。
このような場合は、ローカル変数（説明変数）を活用して式を分解することで、可読性を高めます。

const basePrice = order.quantity * order.itemPrice;
const quantityDiscount = Math.max(0, order.quantity − 500) * order.itemPrice * 0.05; const shipping = Math.min(basePrice * 0.1, 100);
return basePrice − quantityDiscount + shipping;

入力引数は変更前にコピーする

関数の入力引数を変更した場合、副作用を発生させ想定外の事態やバグの原因となることがあります。変更する前にスプレッド演算子などを使用して、新しいデータとしてコピーすることで、元のオブジェクトが変更されるのを防ぐようにしましょう。

  const toConvertKana = (typeKana: string) => {
    let Kana: string[] = [...typeKana]

    const hira = Kana.filter((type) => {
      return type.match(/^[\u3040-\u309f]+$/)
    })

    return hira
  }

メソッドチェーンによるアクセスを絞る

メソッドチェーンとは、「.」で数珠繋ぎにして、要素にアクセスする方法です。下記例のように、メソッドチェーンを使用して、かなり奥深い要素にアクセスできてしまうため、影響範囲が広く、グローバル変数と同様の性質を帯ています。仮にバグが発生した場合、どこでバグが混入したのかを突き止めるため、呼び出し箇所を調べる範囲が拡大してしまいます。できる限り、メソッドチェーンによるアクセスは少なくした方が良いでしょう。

// メソッドチェーン例
export const armor = (memberId: number, newArmor: number) => {
  if (party.members[memberId].equipments.canChange) {
    party.members[memberId].equipments.armor = newArmor
  }
}

マジックバリューを戻り値に使用しない

マジックバリューとは値が存在しないかエラーが発生したことを示すために関数から「-1」を返すことです。

    const getAge = (age: number) => {
      if (age === null) {
        return -1 // 年齢がなければ-1を返す。
      }
      return ageCalculation(age)
    }

基本的にはマジックバリューではなくnullやエラーを返した方が、想定外の事態を起こすリスクを避けることができます。

staticメソッドの適用

staticメソッドは、オブジェクト指向プログラミングにおいて、クラスのインスタンス変数を生成せずに、クラスに直接メソッドやプロパティにアクセスすることができます。インスタンスを生成することなく、処理を共通化させることができる一方で、1つのクラスが複数の責務を持っている状態になるため低凝縮（Low Cohesion）になりやすい特性があります。
その為、凝縮度に影響がない場合、例えば、ログ出力用やフォーマット変換用でstaticメソッドを使用するのに適しています。

class AmountFormatter {
  static formatAmount(amount: string): string | null {
    if (!amount) return null
    
    // 数字以外の文字を削除して数値に変換
    const numericAmount = parseFloat(amount.replace(/[^0-9.-]+/g, ''))
    
    // 3桁区切りの文字列にフォーマット
    const formattedAmount = numericAmount.toLocaleString('ja-JP', {
      style: 'currency',
      currency: 'JPY',
    })
    return formattedAmount
  }
}

横断的感心事は共通化させる

どこにも属すことなく、複数のcomponentで使用するロジック（横断的関心事）は共通化させて、1箇所にまとめておきます。Date整形、ログ出力処理やエラー処理、キャッシュ処理を行うユーティリティなど、同じような処理が多数書かれそうな時は、再利用できる共通処理を実装した共通のクラスやメソッド作成していきます。この場合共通処理用のメソッドはstaticメソッドとして実装されることが多いです。

util/storageUtil.ts

export class StorageUtil {
  static setItem(key: string, value: any): void {
    sessionStorage.setItem(key, JSON.stringify(value));
  }

  static getItem<T>(key: string): T {
    return JSON.parse(sessionStorage.getItem(key) as string) as T;
  }

  static removeItem(key: string): void {
    sessionStorage.removeItem(key);
  }
}

尋ねるな、命じろ（Tell,Don'tAsk）

ソフトウェアの設計において「尋ねるな、命じろ」という格言があります。これは情報隠蔽に関連する設計原則のことで、オブジェクトの内部状態を尋ねたり、その状態に応じて呼び出し側は判断しないということです。呼び出し側はメソッドを通じて処理を命ずるのみで、命令されたで適切な判断や制御をする設計原則です。

この設計原則が守られていな場合は以下のようなイメージになります。

設計原則が守られていない場合

設計原則が守られている場合

この設計原則を守ることで、綺麗なV字となります。これによりオブジェクトの内部構造が隠蔽され、カプセル化が強化されたり、オブジェクト間の依存関係が減少、独立したロジックを持つことでコードの可読性と保守性を向上させることができます。

尋ねるな、命じろ（Tell,Don'tAsk）の原則に基づいた、具体的なコード例は以下の通りです。
銀行口座（BankAccount）クラスと、口座に対する入金・引き出し操作を行うTransactionクラスを定義しています。

class BankAccount {
  private balance: number;

  constructor(initialBalance: number) {
    this.balance = initialBalance;
  }

  public deposit(amount: number): void {
    if (amount > 0) {
      this.balance += amount;
    }
  }

  public withdraw(amount: number): void {
    if (amount > 0 && this.balance >= amount) {
      this.balance -= amount;
    }
  }

  public getBalance(): number {
    return this.balance;
  }
}

class Transaction {
  private account: BankAccount;

  constructor(account: BankAccount) {
    this.account = account;
  }

  public deposit(amount: number): void {
    this.account.deposit(amount);
  }

  public withdraw(amount: number): void {
    this.account.withdraw(amount);
  }
}

// 使用例
const myAccount = new BankAccount(1000);
const myTransaction = new Transaction(myAccount);

myTransaction.deposit(500);
myTransaction.withdraw(200);

console.log(myAccount.getBalance()); // 1300

BankAccount クラスが内部状態（balance）をカプセル化し、その状態に対する操作（deposit および withdraw メソッド）を提供しています。Transaction クラスは BankAccount クラスのインスタンスを操作し、尋ねるのではなく命じることで操作を行います。

コンポーネントの依存関係に注意する

コンポーネント間の依存関係は基本的に単一方向にして、逆方向に依存させないようにしています。

View → Component → Service → Web-serviceといった依存関係があった場合、例えばService → Componentの逆方向で依存関係ができてしまうと、思わぬところでコードの影響範囲が拡大してしまうことになります。

例えば、View側のURLをserviceが取得し処理を分離させたとします。View側で何らかの変更があった時に、serviceの処理も変更させる必要があり、もし他のエンジニアがそのことを知らなければ、予期せぬバグを生み出してしまうことになります。依存関係を守ることでコード修正の影響範囲の把握が容易になり、保守性を高めることができます。

外部のAPI に依存する処理を制限する

外部のAPIにアクセスするのは特定箇所のコードだけに制限するようにします。自分たちでコントロールできないものには依存しすぎないという考えに基づいており、たとえAPIの仕様が変更になったとしても、アクセスしている箇所だけ変更すれば良いので、同様に保守性を高めることができます。

関心事の分離

関心事の分離はよくクリーンアーキテクチャー等で語られますが、それぞれユースケースや目的、役割毎に独立して責任をもち、お互いに影響を与えないようにコードを構成するというものです。もし関心の分離を意識せずに実装を進めた場合、本来関係ない箇所でも修正が入ってしまい、影響範囲が広がります。SOLID原則の一つである、単一責任の原則にも反することになります。クラスやメソッドなどどのような粒子であっても感心事の分離は意識しておきたいところです。

エラーハンドリングの改善

カスタムエラークラスの検討

カスタムエラークラスはErrorクラスを継承して、何らかの想定範囲外のエラーが発生した際に、そのエラーをキャッチして独自のメッセージやコードを出力させる際に使用します。エラーメッセージ形式の統一や独自のエラーコードを作成して処理を共通化させることができます。

エラーが発生した際に、開発者がどこでエラーが発生したか、なぜ処理が失敗したかを素早く理解することができれば、問題の特定が容易になるため、実装はしておきたいところです。

実装方法としては、Errorクラスをextendsで継承して独自のエラークラスを実装します。エラー表示内容を追加したい場合は、constructorに追加していきます。

export class ErrorException extends Error {
  constructor(
      message: string,
      data?: object,
      code?: string,
      type: 'systemError' | 'unknown' | 'warning' | 'validation',
    ) {
    super(message);
  }
}

ErrorExceptionクラスをさらに継承してファクトリー関数を作成します。

export class ExceptionFactory {
　// エラーかどうか
  static isError(error: any): boolean {
    return error.errors && error.errors.length > 0;
  }
  static getException(error: any) {
    if (!this.isError(error)) {
      return;
    }
    return new Exception(
      error.errors.map((err) => ({
        message: err.message,
        type: err.extensions.type,
        data: err.extensions.data,
	code: err.extensions.data
      }))
    );
  }
}

あとは、エラーの場合は出力するようにそれぞれ設定を加えることで出力することができます。

try {
 // 正常な場合
}
catch(error) {
 // エラー
 if (ExceptionFactory.isError(error)) {
  const err = ExceptionFactory.getException(error);
     if (err.type === 'systemError') {
        console.log(err.data);
     }
 }
}

API呼び出しのデザインパターンを検討する

ここだけVue.jsの話になってしまいますが、APIを呼び出す際のデザインパターンを検討する際の個人的なベストプラクティスです（以前書いた個人ブログの記事）。

https://www.multispots.net/repository-factory-pattern/

APIを呼び出す際にコンポーネント内にaxiosインスタンスを直書きしてAPIを呼び出した場合、単体テストやAPIの呼び出し処理の再利用が困難であり、それを解消するために、リポジトリファクトリパターンを採用します。

ドメイン毎にデータにアクセスする処理をRepositoryに集約するファイルを作成して、APIのリクエスト処理をまとめます。

import { NuxtAxiosInstance } from '@nuxtjs/axios'
import { AxiosResponse, AxiosInstance } from 'axios'

export type Book = {
  id: number
  title: string
  mail: string
}

export type BookRepository = {
  get: () => Promise<AxiosResponse<{ shops: Book[] }>>
  findById: (id: number) => Promise<AxiosResponse<Book>>
}

export const BooksRepository = (
  $axios: NuxtAxiosInstance | AxiosInstance
): BookRepository => ({
  get: () => {
    return $axios.get(`/books`)
  },
  findById: (id) => {
    return $axios.get(`${id}`)
  },
})

Factoryも作成して、どのRepositoryを生成するかはFactoryが決定できるようにしておきます。

import { UserRepository } from '../repositories/userRepository'
import { BooksRepository } from '../repositories/booksRepository'

export interface Repositories {
  user: typeof UserRepository
  books: typeof BooksRepository
}

const repositories: Repositories = {
  user: UserRepository,
  books: BooksRepository,
}

export const apiRepositoryFactory = {
  get: (name: keyof Repositories) => repositories[name],
}

APIの呼び出しコードをコンポーネントに直書きするのではなく、呼び出し元を集約して記載することでメンテナンス性が向上し、拡張性の高い実装を行うことができます。

さいごに

ネストを浅くすることやイミュータブルの検討に関することなど、色々記載しようかと思いましたが、見返してみるとクリーンアーキテクチャー成分が多めとなってしまいました。何か間違いなどがあればご指摘いただけると助かります。

文献
https://qiita.com/TakeshiFukushima/items/0b16aef3d320a2ccb7c3

https://www.amazon.co.jp/Good-Code-Bad-～持続可能な開発のためのソフトウェアエンジニア的思考-ebook/dp/B0BSW72QKZ/ref=sr_1_1?__mk_ja_JP=カタカナ&crid=T6HX1MV1BOA2&keywords=good+code&qid=1681829003&s=digital-text&sprefix=good+code%2Cdigital-text%2C242&sr=1-1

https://www.amazon.co.jp/良いコード／悪いコードで学ぶ設計入門―保守しやすい-成長し続けるコードの書き方-仙塲-大也-ebook/dp/B09Y1MWK9N/ref=sr_1_1?crid=3PIVDS1NRXGZ&keywords=良いコード%2F悪いコードで学ぶ設計入門&qid=1681829023&s=digital-text&sprefix=良いコード%2Cdigital-text%2C245&sr=1-1

https://www.amazon.co.jp/Clean-Architecture-達人に学ぶソフトウェアの構造と設計-アスキードワンゴ-Ｒｏｂｅｒｔ-Ｃ．Ｍａｒｔｉｎ-ebook/dp/B07FSBHS2V/ref=sr_1_2?crid=WMSV6WJF4MA&keywords=クリーンコート&qid=1681829040&s=digital-text&sprefix=クリーン%2Cdigital-text%2C230&sr=1-2