実践！AWS CDK #9 リファクタリング

はじめに

今回はリファクタリングを実施します。
サブネット生成に関するコードをキレイにしていきましょう。

前回の記事はこちら。

既存の作り

現在のサブネット生成コードはこうなっています。

import * as cdk from '@aws-cdk/core';
import { CfnSubnet, CfnVPC } from '@aws-cdk/aws-ec2';
import { Resource } from './abstract/resource';

export class Subnet extends Resource {
    public public1a: CfnSubnet;
    public public1c: CfnSubnet;
    public app1a: CfnSubnet;
    public app1c: CfnSubnet;
    public db1a: CfnSubnet;
    public db1c: CfnSubnet;

    private readonly vpc: CfnVPC;

    constructor(vpc: CfnVPC) {
        super();
        this.vpc = vpc;
    };

    createResources(scope: cdk.Construct) {
        this.public1a = new CfnSubnet(scope, 'SubnetPublic1a', {
            cidrBlock: '10.0.11.0/24',
            vpcId: this.vpc.ref,
            availabilityZone: 'ap-northeast-1a',
            tags: [{ key: 'Name', value: this.createResourceName(scope, 'subnet-public-1a') }]
        });
        this.public1c = new CfnSubnet(scope, 'SubnetPublic1c', {
            cidrBlock: '10.0.12.0/24',
            vpcId: this.vpc.ref,
            availabilityZone: 'ap-northeast-1c',
            tags: [{ key: 'Name', value: this.createResourceName(scope, 'subnet-public-1c') }]
        });
        this.app1a = new CfnSubnet(scope, 'SubnetApp1a', {
            cidrBlock: '10.0.21.0/24',
            vpcId: this.vpc.ref,
            availabilityZone: 'ap-northeast-1a',
            tags: [{ key: 'Name', value: this.createResourceName(scope, 'subnet-app-1a') }]
        });
        this.app1c = new CfnSubnet(scope, 'SubnetApp1c', {
            cidrBlock: '10.0.22.0/24',
            vpcId: this.vpc.ref,
            availabilityZone: 'ap-northeast-1c',
            tags: [{ key: 'Name', value: this.createResourceName(scope, 'subnet-app-1c') }]
        });
        this.db1a = new CfnSubnet(scope, 'SubnetDb1a', {
            cidrBlock: '10.0.31.0/24',
            vpcId: this.vpc.ref,
            availabilityZone: 'ap-northeast-1a',
            tags: [{ key: 'Name', value: this.createResourceName(scope, 'subnet-db-1a') }]
        });
        this.db1c = new CfnSubnet(scope, 'SubnetDb1c', {
            cidrBlock: '10.0.32.0/24',
            vpcId: this.vpc.ref,
            availabilityZone: 'ap-northeast-1c',
            tags: [{ key: 'Name', value: this.createResourceName(scope, 'subnet-db-1c') }]
        });
    }
}

一見、妥協できそうな作りではありますが CfnSubnet の初期化処理が繰り返し（6 回も！）実行されており、設定するプロパティの変動部分と固定部分がわかりにくくなっています。この点に関して改善していきましょう。

実装

リファクタリングの結果はこちらです。

import * as cdk from '@aws-cdk/core';
import { CfnSubnet, CfnVPC } from '@aws-cdk/aws-ec2';
import { Resource } from './abstract/resource';

interface ResourceInfo {
    readonly id: string;
    readonly cidrBlock: string;
    readonly availabilityZone: string;
    readonly resourceName: string;
    readonly assign: (subnet: CfnSubnet) => void;
}

export class Subnet extends Resource {
    public public1a: CfnSubnet;
    public public1c: CfnSubnet;
    public app1a: CfnSubnet;
    public app1c: CfnSubnet;
    public db1a: CfnSubnet;
    public db1c: CfnSubnet;

    private readonly vpc: CfnVPC;
    private readonly resourcesInfo: ResourceInfo[] = [
        {
            id: 'SubnetPublic1a',
            cidrBlock: '10.0.11.0/24',
            availabilityZone: 'ap-northeast-1a',
            resourceName: 'subnet-public-1a',
            assign: subnet => this.public1a = subnet
        },
        {
            id: 'SubnetPublic1c',
            cidrBlock: '10.0.12.0/24',
            availabilityZone: 'ap-northeast-1c',
            resourceName: 'subnet-public-1c',
            assign: subnet => this.public1c = subnet
        },
        {
            id: 'SubnetApp1a',
            cidrBlock: '10.0.21.0/24',
            availabilityZone: 'ap-northeast-1a',
            resourceName: 'subnet-app-1a',
            assign: subnet => this.app1a = subnet
        },
        {
            id: 'SubnetApp1c',
            cidrBlock: '10.0.22.0/24',
            availabilityZone: 'ap-northeast-1c',
            resourceName: 'subnet-app-1c',
            assign: subnet => this.app1c = subnet
        },
        {
            id: 'SubnetDb1a',
            cidrBlock: '10.0.31.0/24',
            availabilityZone: 'ap-northeast-1a',
            resourceName: 'subnet-db-1a',
            assign: subnet => this.db1a = subnet
        },
        {
            id: 'SubnetDb1c',
            cidrBlock: '10.0.32.0/24',
            availabilityZone: 'ap-northeast-1c',
            resourceName: 'subnet-db-1c',
            assign: subnet => this.db1c = subnet
        }
    ];

    constructor(vpc: CfnVPC) {
        super();
        this.vpc = vpc;
    };

    createResources(scope: cdk.Construct) {
        for (const resourceInfo of this.resourcesInfo) {
            const subnet = this.createSubnet(scope, resourceInfo);
            resourceInfo.assign(subnet);
        }
    }

    private createSubnet(scope: cdk.Construct, resourceInfo: ResourceInfo): CfnSubnet {
        const subnet = new CfnSubnet(scope, resourceInfo.id, {
            cidrBlock: resourceInfo.cidrBlock,
            vpcId: this.vpc.ref,
            availabilityZone: resourceInfo.availabilityZone,
            tags: [{ key: 'Name', value: this.createResourceName(scope, resourceInfo.resourceName) }]
        });

        return subnet;
    }
}

30 行ほど増えちゃいました（笑）
しかし長いところは変動部分の宣言であり、リソースの生成処理自体は 15 行程度に収まっています。ここは人によって好みが分かれそうですが、私はこの方針で進めたいと思います。

順に見ていきましょう。

解説

まずは CfnSubnet の初期化処理から。

private createSubnet(scope: cdk.Construct, resourceInfo: ResourceInfo): CfnSubnet {
    const subnet = new CfnSubnet(scope, resourceInfo.id, {
        cidrBlock: resourceInfo.cidrBlock,
        vpcId: this.vpc.ref,
        availabilityZone: resourceInfo.availabilityZone,
        tags: [{ key: 'Name', value: this.createResourceName(scope, resourceInfo.resourceName) }]
    });

    return subnet;
}

繰り返し行われていた CfnSubnet のインスタンス生成処理を createSubnet() というメソッドに切り出しました。引数として各プロパティに設定する変動部分（resourceInfo）を受け取り、設定したい値を取得します。なお設定項目の 1 つ vpcId は常に固定（this.vpc.ref）です。このクラスでしか利用しないので private メソッドにします。

• [参考]：class CfnSubnet (construct) | AWS CDK API Reference

以下はその変動部分を定義する構造体の宣言です。

interface ResourceInfo {
    readonly id: string;
    readonly cidrBlock: string;
    readonly availabilityZone: string;
    readonly resourceName: string;
    readonly assign: (subnet: CfnSubnet) => void;
}

「リソースごとに異なるプロパティのみ」を抜き出した構造体を interface で定義しています。変更防止のため各要素を readonly とします。

最後の要素 assign に関してはまた後ほど。

• [参考]：Interfaces | TypeScript: Documentation

そして実際のプロパティ値を設定している部分がこちら。

private readonly resourcesInfo: ResourceInfo[] = [
    {
        id: 'SubnetPublic1a',
        cidrBlock: '10.0.11.0/24',
        availabilityZone: 'ap-northeast-1a',
        resourceName: 'subnet-public-1a',
        assign: subnet => this.public1a = subnet
    },
    {
        id: 'SubnetPublic1c',
        cidrBlock: '10.0.12.0/24',
        availabilityZone: 'ap-northeast-1c',
        resourceName: 'subnet-public-1c',
        assign: subnet => this.public1c = subnet
    },
    {
        id: 'SubnetApp1a',
        cidrBlock: '10.0.21.0/24',
        availabilityZone: 'ap-northeast-1a',
        resourceName: 'subnet-app-1a',
        assign: subnet => this.app1a = subnet
    },
    {
        id: 'SubnetApp1c',
        cidrBlock: '10.0.22.0/24',
        availabilityZone: 'ap-northeast-1c',
        resourceName: 'subnet-app-1c',
        assign: subnet => this.app1c = subnet
    },
    {
        id: 'SubnetDb1a',
        cidrBlock: '10.0.31.0/24',
        availabilityZone: 'ap-northeast-1a',
        resourceName: 'subnet-db-1a',
        assign: subnet => this.db1a = subnet
    },
    {
        id: 'SubnetDb1c',
        cidrBlock: '10.0.32.0/24',
        availabilityZone: 'ap-northeast-1c',
        resourceName: 'subnet-db-1c',
        assign: subnet => this.db1c = subnet
    }
];

上記で宣言したインタフェース ResourceInfo 型を要素に持つ配列です。
作成したいサブネットのリソース情報が記述されています。つまりここを見るだけでどのようなリソースを作るのかが一覧で確認できる作りになっています。
見やすい！（が、行数は多い）

短い行で書くこともできるのですが、インデントが揃わないため私は上記を採用しました。

private readonly resourcesInfo: ResourceInfo[] = [
    { id: 'SubnetPublic1a', cidrBlock: '10.0.11.0/24', availabilityZone: 'ap-northeast-1a', resourceName: 'subnet-public-1a', assign: subnet => this.public1a = subnet },
    { id: 'SubnetPublic1c', cidrBlock: '10.0.12.0/24', availabilityZone: 'ap-northeast-1c', resourceName: 'subnet-public-1c', assign: subnet => this.public1c = subnet },
    { id: 'SubnetApp1a', cidrBlock: '10.0.21.0/24', availabilityZone: 'ap-northeast-1a', resourceName: 'subnet-app-1a', assign: subnet => this.app1a = subnet },
    { id: 'SubnetApp1c', cidrBlock: '10.0.22.0/24', availabilityZone: 'ap-northeast-1c', resourceName: 'subnet-app-1c', assign: subnet => this.app1c = subnet },
    { id: 'SubnetDb1a', cidrBlock: '10.0.31.0/24', availabilityZone: 'ap-northeast-1a', resourceName: 'subnet-db-1a', assign: subnet => this.db1a = subnet },
    { id: 'SubnetDb1c', cidrBlock: '10.0.32.0/24', availabilityZone: 'ap-northeast-1c', resourceName: 'subnet-db-1c', assign: subnet => this.db1c = subnet }
];

そして以下が配列の要素数だけループ処理を実行するコードになります。

createResources(scope: cdk.Construct) {
    for (const resourceInfo of this.resourcesInfo) {
        const subnet = this.createSubnet(scope, resourceInfo);
        resourceInfo.assign(subnet);
    }
}

• [参考]：for..of statements | TypeScript: Documentation

実行している処理は次の 2 点

1. サブネットリソースの生成
2. 生成したオブジェクトを自身のメンバ変数に代入

メンバ変数への代入で重要なのが、先ほど説明を後回しにした ResourceInfo インタフェースの最終要素 assign です。

interface ResourceInfo {
    readonly id: string;
    readonly cidrBlock: string;
    readonly availabilityZone: string;
    readonly resourceName: string;
    readonly assign: (subnet: CfnSubnet) => void;
}

こちらは動的な処理を実行するために 関数 として宣言しています。引数は CfnSubnet 型、戻り値は void です。また、簡略化のためアロー関数としています。

• [参考]：Arrow Functions | TypeScript: Documentation

実際の代入処理はこちら。

private readonly resourcesInfo: ResourceInfo[] = [
    {
        id: 'SubnetPublic1a',
        cidrBlock: '10.0.11.0/24',
        availabilityZone: 'ap-northeast-1a',
        resourceName: 'subnet-public-1a',
        assign: subnet => this.public1a = subnet
    },
    ~ 省略 ~

引数として受け取った CfnSubnet 型のオブジェクト subnet を this.public1a に代入するという処理です。このように関数をうまく構造体に組み込むことで動的な処理が実現可能となります。

この関数がないとこんな感じになっちゃうんですよ。

createResources(scope: cdk.Construct) {
    this.public1a = this.createSubnet(scope, this.resourcesInfo[0]);
    this.public1c = this.createSubnet(scope, this.resourcesInfo[1]);
    this.app1a = this.createSubnet(scope, this.resourcesInfo[2]);
    this.app1c = this.createSubnet(scope, this.resourcesInfo[3]);
    this.db1a = this.createSubnet(scope, this.resourcesInfo[4]);
    this.db1c = this.createSubnet(scope, this.resourcesInfo[5]);
}

0, 1, ... という添字部分がちょっと気持ち悪いですね。

ちなみにこのやり方（インタフェース内の関数指定）を教えてくれたのは私のオトモダチ yoshihitoh さんです。
ありがとう！

テスト

リファクタリング後は必ずテストを実行しましょう。

npm run build && npm test

> devio@0.1.0 build
> tsc


> devio@0.1.0 test
> jest

 PASS  test/devio.test.ts
 PASS  test/resource/vpc.test.ts
 PASS  test/resource/subnet.test.ts

Test Suites: 3 passed, 3 total
Tests:       3 passed, 3 total
Snapshots:   0 total
Time:        2.315 s
Ran all test suites.

OK です！

GitHub

今回のソースコードは コチラ です。

おわりに

結果的に行数は増えてしまいましたが、ロジック部分が短くシンプルになったので私としては満足です。

さて、これまで長い道のりでしたがようやくプログラムの土台が完成しました。今後は以下のルールに従ってガンガン（と言ってものんびりと）リソースを作っていきたいと思います。

• 作成するリソースごとにファイル（クラス）を分割する
  • 例）internetGateway.ts（InternetGateway クラス）
• すべてのリソースクラスは抽象クラス Resource を継承する
• 「あるリソースの生成に必要となるリソース」はコンストラクタに渡す
  • 渡すオブジェクトは CfnXXX クラスのインスタンス
• リソースクラス内で複数のリソースを生成する場合はループで回す
  • new CfnXXX() を何度も実行しない
  • ResourceInfo インタフェースを用意してプロパティの変動部分のみを書き出す
• 生成したリソースはリソースクラス内の public メンバ変数に格納し、外部クラスから参照可能とする

こんなところですね。
これで進めてみて、困ったことが出てきたらその時に考えようと思います。

リンク

• class CfnSubnet (construct) | AWS CDK API Reference
• Everyday Types | TypeScript: Documentation
• Iterators and Generators | TypeScript: Documentation
• Classes | TypeScript: Documentation