📝

DRYを間違って適応するとどうなるのか

2021/09/21に公開1件

TypeScript

DRY(Don't Repeat Yourself!)原則ありますね。昔はコードの二重化をなくすことと考えられていましたが、現在では「知識」の二重化なくす原則と考えられています。達人プログラマーでも以下のように述べられています。

コードの二重化すべてが知識の二重化というわけではないワインの注文アプリで、顧客の年齢を認証し、注文本数を受け付けるモジュールを開発していると考えてください。

def  validate_age(value):
  validate_type(value,:integer)
  validate_min_integer(value,0)

def validate_quantity(value):
  validate_type(value,:integer)
  validate_min_integer(value,0)

コードレビューの際に、これら2つの関数のコードは同じであるため、DRY原則に違反しているという声が上がりました。しかしその意見は間違っています。コードは同じですが、これらコードが表現している知識は異なっているのです。これら2つの関数は、異なる2つのものごとが同じ規則を有しているということを示しているだけです。それは偶然であり二重化ではありません。

デイビット・トーマス,アンドリュー・ハント. 達人プログラマー熟達に向けたあなたの旅第2版 (Japanese Edition) (Kindle Locations 1315-1329). Kindle Edition.

この例を他にも考えてみました。テーマは営業と技術職の社員のモデルです。

例は以下の４つから成り立っています。

最初の構成
DRYを適応しクラスを統合する
クラスが成長する
再びクラスの分離を試みる

4のサブクラス化の過程で継承と委譲両方のコードサンプルを提示しています。

なおタイトルは間違って適応するとと書きましたが、現時点のモデルが知識の二重化なのか単にコードが偶然に一致しているのかの判断は難しいです(UMLモデリングからはある程度推測はできますが)。間違ったDRYなのか気にするよりかは、テストスイートを充実させて定期的にリファクタリングするほうがよいプラクティスだと思います。

なおサンプルはTypeScriptです。

最初の構成

最初こんな感じでした。営業と技術職それぞれのクラスがあり同じデータと同じふるまいを持ちます。

営業クラス sales.ts

export class Sales {
  private firstName: string;

  private lastName: string;

  private employeeId: string;

  constructor(args: {
    firstName: string;
    lastName: string;
    employeeId: string;
  }) {
    this.firstName = args.firstName;
    this.lastName = args.lastName;
    this.employeeId = args.employeeId;
  }

  startJob() {
    console.log('Hello!');
  }

  doSelfIntroduction() {
    console.log(`I\'m ${this.firstName}${this.lastName}`);
  }

  finishJob() {
    console.log('Bye!');
  }
}

技術職クラス technicalStaff.ts

export class TechnicalStaff {
  private firstName: string;

  private lastName: string;

  private employeeId: string;

  constructor(args: {
    firstName: string;
    lastName: string;
    employeeId: string;
  }) {
    this.firstName = args.firstName;
    this.lastName = args.lastName;
    this.employeeId = args.employeeId;
  }

  startJob() {
    console.log('Hello!');
  }

  doSelfIntroduction() {
    console.log(`I\'m ${this.firstName}${this.lastName}`);
  }

  finishJob() {
    console.log('Bye!');
  }
}

index.ts

import { Sales } from './sales';
import { TechnicalStaff } from './technicalStaff';

const aSales = new Sales({
  firstName: '太郎',
  lastName: '日本',
  employeeId: 'em_fakljjefklajfi',
});

const aTechnicalStaff = new TechnicalStaff({
  firstName: '花子',
  lastName: '日本',
  employeeId: 'em_jfakegahrjghj',
});

// 自己紹介
aSales.doSelfIntroduction();
aTechnicalStaff.doSelfIntroduction();

// 仕事開始
aSales.startJob();
aTechnicalStaff.startJob();

// 仕事終わり
aSales.finishJob();
aTechnicalStaff.finishJob();

DRYを適応しクラスを統合する

同じ内容なのでクラスを統合してEmployeeクラスにしました。スッキリしましたね。

社員 employee.ts

export class Employee {
  private firstName: string;

  private lastName: string;

  private employeeId: string;

  private type: 'SALES' | 'TECHNICAL_STAFF';

  constructor(args: {
    firstName: string;
    lastName: string;
    employeeId: string;
    type: 'SALES' | 'TECHNICAL_STAFF';
  }) {
    this.firstName = args.firstName;
    this.lastName = args.lastName;
    this.employeeId = args.employeeId;
    this.type = args.type;
  }

  startJob() {
    console.log('Hello!');
  }

  doSelfIntroduction() {
    console.log(`I\'m ${this.firstName}${this.lastName}`);
  }

  finishJob() {
    console.log('Bye!');
  }
}

index.ts

import { Employee } from './employee';
const aSales = new Employee({
  firstName: '太郎',
  lastName: '日本',
  employeeId: 'em_fakljjefklajfi',
  type: 'SALES',
});

const aTechnicalStaff = new Employee({
  firstName: '花子',
  lastName: '日本',
  employeeId: 'em_jfakegahrjghj',
  type: 'TECHNICAL_STAFF',
});

aSales.doSelfIntroduction();
aTechnicalStaff.doSelfIntroduction();

aSales.startJob();
aTechnicalStaff.startJob();

aSales.finishJob();
aTechnicalStaff.finishJob();

クラスが成長する

さてここでシステムが成長しクラスが成長したとします。Employeeクラスは次の用になってしまいました。営業か技術職かどうかで、分岐が多くなったのです。また新たにdoTechnicalJobとdoSalesJobのようにそれぞれの専用メソッドも現れ始めました。このコード臭いますねぇ。

社員 employee.ts

export class Employee {
  private firstName: string;

  private lastName: string;

  private employeeId: string;

  private type: 'SALES' | 'TECHNICAL_STAFF';

  private technicalTool?: string; // type === TECHNICAL_STAFF のみが使用

  private salesTool?: string; // type === SALES のみが使用

  constructor(args: {
    firstName: string;
    lastName: string;
    employeeId: string;
    type: 'SALES' | 'TECHNICAL_STAFF';
    technicalTool?: string;
    salesTool?: string;
  }) {
    this.firstName = args.firstName;
    this.lastName = args.lastName;
    this.employeeId = args.employeeId;
    this.type = args.type;
    this.technicalTool = args.technicalTool;
    this.salesTool = args.salesTool;
  }

  startJob() {
    console.log('Hello!');
  }

  doSelfIntroduction() {
    if (this.type === 'SALES') {
      console.log(
        `I\'m ${this.firstName}${this.lastName}! I'm belongs to a sales team!`
      );
    } else if (this.type === 'TECHNICAL_STAFF') {
      console.log(
        `I\'m ${this.firstName}${this.lastName}! I'm belongs to a technical staff team!`
      );
    }
  }

  doTechnicalJob() {
    if (this.type !== 'TECHNICAL_STAFF') return;

    if (!this.technicalTool)
      throw new Error(
        `Error in Employee.doTechnicalJob(). Technical tool is undefined!`
      );

    console.log(`Using ${this.technicalTool}!`);
  }

  doSalesJob() {
    if (this.type !== 'SALES') return;

    if (!this.salesTool)
      throw new Error(
        `Error in Employee.doSalesJob(). Sales tool is undefined!`
      );

    console.log(`Using ${this.salesTool}!`);
  }

  finishJob() {
    console.log('Bye!');
  }
}

index.ts

import { Employee } from './employee';
const aSales = new Employee({
  firstName: '太郎',
  lastName: '日本',
  employeeId: 'em_fakljjefklajfi',
  type: 'SALES',
  salesTool: 'sugoi-sales-tool',
});

const aTechnicalStaff = new Employee({
  firstName: '花子',
  lastName: '日本',
  employeeId: 'em_jfakegahrjghj',
  type: 'TECHNICAL_STAFF',
  technicalTool: 'sugoi-technical-tool',
});

aSales.doSelfIntroduction();
aTechnicalStaff.doSelfIntroduction();

aSales.startJob();
aTechnicalStaff.startJob();

// 新規に追加
// それぞれの仕事をする
aSales.doSalesJob();
aTechnicalStaff.doTechnicalJob();

aSales.finishJob();
aTechnicalStaff.finishJob();

再びクラスの分離を試みる

ということでリファクタリングをします。はじめは営業と技術職にそれぞれ特有の振る舞いはなかったので良かったのですが、モデルが成長するにつれてそれぞれの個性が出始めました。そこで再び分離することにします。

修正の仕方は3つあります。

継承を利用する
委譲を利用する(パターン１)
委譲を利用する(パターン２)

継承を利用する

社員(Employee)クラスを基底に、営業と技術職を作ります。継承先のクラスにはそれぞれ独自のメソッドを定義します。

社員 employee.ts

export class Employee {
  protected firstName: string;

  protected lastName: string;

  protected employeeId: string;

  constructor(args: {
    firstName: string;
    lastName: string;
    employeeId: string;
  }) {
    this.firstName = args.firstName;
    this.lastName = args.lastName;
    this.employeeId = args.employeeId;
  }

  startJob() {
    console.log('Hello!');
  }

  doSelfIntroduction() {
    throw new Error(`Don't call me directory! Please use extend class!`);
  }

  finishJob() {
    console.log('Bye!');
  }
}

営業 sales.ts

import { Employee } from './employee';

export class Sales extends Employee {
  private salesTool: string;

  constructor(args: {
    firstName: string;
    lastName: string;
    employeeId: string;
    salesTool: string;
  }) {
    super(args);

    this.salesTool = args.salesTool;
  }

  doSelfIntroduction() {
    console.log(
      `I\'m ${this.firstName}${this.lastName}! I'm belongs to a sales team!`
    );
  }

  doSalesJob() {
    console.log(`Using ${this.salesTool}!`);
  }
}

技術職 technicalStaff.ts

import { Employee } from './employee';

export class TechnicalStaff extends Employee {
  private technicalTool: string;

  constructor(args: {
    firstName: string;
    lastName: string;
    employeeId: string;
    technicalTool: string;
  }) {
    super(args);

    this.technicalTool = args.technicalTool;
  }

  doSelfIntroduction() {
    console.log(
      `I\'m ${this.firstName}${this.lastName}! I'm belongs to a technical staff team!`
    );
  }

  doTechnicalJob() {
    console.log(`Using ${this.technicalTool}!`);
  }
}

index.ts

import { Sales } from './sales';
import { TechnicalStaff } from './technicalStaff';

const aSales = new Sales({
  firstName: '太郎',
  lastName: '日本',
  employeeId: 'em_fakljjefklajfi',
  salesTool: 'lead-customer-list',
});

const aTechnicalStaff = new TechnicalStaff({
  firstName: '花子',
  lastName: '日本',
  employeeId: 'em_jfakegahrjghj',
  technicalTool: 'spanner',
});

aSales.doSelfIntroduction();
aTechnicalStaff.doSelfIntroduction();

aSales.startJob();
aTechnicalStaff.startJob();

aSales.doSalesJob();
aTechnicalStaff.doTechnicalJob();

aSales.finishJob();
aTechnicalStaff.finishJob();

委譲を利用する(パターン1)

extendsの代わりにimplementsを使います。interfaceをimplementsする、３つのメソッドを実装しないとコンパイルエラーになります。

社員インターフェース employeeInterface.ts

export interface Employee {
  startJob: () => void;
  finishJob: () => void;
  doSelfIntroduction: () => void;
}

営業 sales.ts

import { Employee } from './employeeInterface';

export class Sales implements Employee {
  private firstName: string;

  private lastName: string;

  private employeeId: string;

  private salesTool: string;

  constructor(args: {
    firstName: string;
    lastName: string;
    employeeId: string;
    salesTool: string;
  }) {
    this.firstName = args.firstName;
    this.lastName = args.lastName;
    this.employeeId = args.employeeId;
    this.salesTool = args.salesTool;
  }

  startJob() {
    console.log('Hello!');
  }

  doSelfIntroduction() {
    console.log(
      `I\'m ${this.firstName}${this.lastName}! I'm belongs to a sales team!`
    );
  }

  finishJob() {
    console.log('Bye!');
  }

  doSalesJob() {
    console.log(`Using ${this.salesTool}!`);
  }
}

技術職 technicalStaff

import { Employee } from './employeeInterface';

export class TechnicalStaff implements Employee {
  private firstName: string;

  private lastName: string;

  private employeeId: string;

  private technicalTool: string;

  constructor(args: {
    firstName: string;
    lastName: string;
    employeeId: string;
    technicalTool: string;
  }) {
    this.firstName = args.firstName;
    this.lastName = args.lastName;
    this.employeeId = args.employeeId;
    this.technicalTool = args.technicalTool;
  }

  startJob() {
    console.log('Hello!');
  }

  doSelfIntroduction() {
    console.log(
      `I\'m ${this.firstName}${this.lastName}! I'm belongs to a technical staff team!`
    );
  }

  finishJob() {
    console.log('Bye!');
  }

  doTechnicalJob() {
    console.log(`Using ${this.technicalTool}!`);
  }
}

index.ts

import { Sales } from './sales';
import { TechnicalStaff } from './technicalStaff';

const aSales = new Sales({
  firstName: '太郎',
  lastName: '日本',
  employeeId: 'em_fakljjefklajfi',
  salesTool: 'lead-customer-list',
});

const aTechnicalStaff = new TechnicalStaff({
  firstName: '花子',
  lastName: '日本',
  employeeId: 'em_jfakegahrjghj',
  technicalTool: 'spanner',
});

aSales.doSelfIntroduction();
aTechnicalStaff.doSelfIntroduction();

aSales.startJob();
aTechnicalStaff.startJob();

aSales.doSalesJob();
aTechnicalStaff.doTechnicalJob();

aSales.finishJob();
aTechnicalStaff.finishJob();

委譲を利用する(パターン2)

パターン１では、startJobやfinishJobがほぼ同じでした。そこでそれぞれのクラス特有の振る舞いを持つ部分だけをinterfaceで定義して、その部分のみを別クラスにして実装したいと思います。また共通化にあたりdoSalesJobとdoTechnicalJobをdoJobとして統一しています。

委譲という言葉的にはパターン１よりもパターン２のほうが振る舞いを渡しているのでしっくりきますね(パターン１は委譲とは言わないのかな？誰か教えて下さい)。

Jobインターフェース jobInterface.ts

export interface Job {
  doSelfIntroduction: (args: { firstName: string; lastName: string }) => void;
  doJob: () => void;
}

社員 employee.ts

import { Job } from './jobInterface';

export class Employee {
  private firstName: string;

  private lastName: string;

  private employeeId: string;

  private job: Job;

  constructor(args: {
    firstName: string;
    lastName: string;
    employeeId: string;
    job: Job;
  }) {
    this.firstName = args.firstName;
    this.lastName = args.lastName;
    this.employeeId = args.employeeId;
    this.job = args.job;
  }

  startJob() {
    console.log('Hello!');
  }

  doSelfIntroduction() {
    this.job.doSelfIntroduction({
      firstName: this.firstName,
      lastName: this.lastName,
    });
  }

  doJob() {
    this.job.doJob();
  }

  finishJob() {
    console.log('Bye!');
  }
}

営業Job salesJob.ts

import { Job } from './jobInterface';

export class SalesJob implements Job {
  private salesTool: string;

  constructor(args: { salesTool: string }) {
    this.salesTool = args.salesTool;
  }

  doSelfIntroduction(args: { firstName: string; lastName: string }) {
    console.log(
      `I\'m ${args.firstName}${args.lastName}! I'm belongs to a sales team!`
    );
  }

  doJob() {
    console.log(`Using ${this.salesTool}!`);
  }
}

技術職Job technicalStaffJob.ts

import { Job } from './jobInterface';

export class TechicalStaffJob implements Job {
  private technicalTool: string;

  constructor(args: { technicalTool: string }) {
    this.technicalTool = args.technicalTool;
  }

  doSelfIntroduction(args: { firstName; string; lastName: string }) {
    console.log(
      `I\'m ${args.firstName}${args.lastName}! I'm belongs to a technical staff team!`
    );
  }

  doJob() {
    console.log(`Using ${this.technicalTool}!`);
  }
}

index.ts

import { SalesJob } from './salesJob';
import { TechnicalStaffJob } from './technicalStaffJob';
import { Employee } from './employee';

const aSales = new Employee({
  firstName: '太郎',
  lastName: '日本',
  employeeId: 'em_fakljjefklajfi',
  job: new SalesJob({
    salesTool: 'lead-customer-list',
  }),
});

const aTechnicalStaff = new Employee({
  firstName: '花子',
  lastName: '日本',
  employeeId: 'em_jfakegahrjghj',
  job: new TechnicalStaffJob({
    technicalTool: 'spanner',
  }),
});

aSales.doSelfIntroduction();
aTechnicalStaff.doSelfIntroduction();

aSales.startJob();
aTechnicalStaff.startJob();

aSales.doJob();
aTechnicalStaff.doJob();

aSales.finishJob();
aTechnicalStaff.finishJob();

Discussion

ハトすけ 2021/09/29

やりすぎたDRYってモデルを見ずにコードだけみたときに起きる気がする。まずUMLの概念モデリングから入れば、異なる知識を同一するみたいなことは起きにくくなる。