初めに
美しいコードとはどのようなものなのだろうか、、?
自分は学生で、同じインターンに参加する後輩のコードをリファクタリングであったり方向性だったりを指示することがある。
自分程度が指導するなどおこがましいとは思いつつ、何とか教えている現状である。
しかし、美しいコード
とはいったいどのようなものであるのだろうか、、?
知り合いの中では、「美しいコード
が好きです。」「汚いコード
は嫌いです。」といった人も存在する。
そのような人は、無駄のない効率的なコード
(行・文字数が少ない、高速で動作する)を書く傾向にあると感じる。
しかし、効率的
=美しい
であるのだろうか?
美しい
とはいったいどのようなものであるのかを踏まえて、言語化して落とし込んでいこうと思う。
断っておくが、あくまでも学生の戯言であり、現場における最適化されたコードや限りあるリソースの中で生じたコードに対して異を唱えるわけではない。
あくまでも、現状の自分の持つ美しいコード像
を言語化することを目的としていることを念頭に置いてほしい。
美しいコードとはどのようなものを指すのか
上の記事では、世間一般の美しさの法則
から、コードにおける美しさにおいて必要なことを示している。最低限美
を実現するのに必要なことが書いてあるように見えるため、一読するとよいかもしれない。
上記の記事を読んで、私はこのように感じた
確かに重要だ。でも、いったいどうしたらよいというのだろうか。
記事の内容は正に重要なことがまとまっており、これらすべてを実現できるのであれば、それは究極的な美を象徴するコード
となると思う。
しかし、芸術における美しさ
を私が理解できず再現できないように、究極的に完成された美しさ
を持つコードは論理化し、実践知に落とさなければ実現は不可能であると考える。
リーダブルコードに関しても同様なのだ。ある種コーダーは経験から知っていて、当たり前のことを数多く言語化しており、それが実現できれば良いコード
に大幅に近づくことができると分かる。
重要なのは理解できるが、実践まで落とし込めないのである。
理論は理解できても、実現できないからこそ、多くの熟練の技術者をも巻き込む困難な問題となっているのである。
一方で救いとなる点をあげれば、コードにおける美しさ
に関しては、すでにある程度答えが出ていると考えている。
コードが生き物である以上、不定形なものであるが、抑えなければ一気に汚いコード
になってしまうポイントというものは存在しているといえる。
このポイントを抑えれば、致命的な汚いコード
とはなりえないだろう。
美しい
コードは文脈に依存する
先ほど美しいコードに関してはある程度答えが出ている
と述べたがあれは誤りでもある。
なぜなら、人が美しいと感じる観点は様々であるからである。
例えば偉大な建造物があるときに、その伝統的な工法に対して感慨を得るのか、昔の人の圧倒的な知恵、歴史的な価値...etcと、どの点に着目して美しさ
を見出すかは不明であるからだ。
例えば、
- 効率的なコードが美しいと思う人
- 文字数が圧倒的に少ないコードが美しいと思う人
- 限りなく抽象化されたコードが美しいと思う人
- ...etc
と、同じコードに対しても美しい
・汚い
と感じる人はいるだろう
このような美しさ
に対する考え方が異なる人々が同じチームにいる場合に困難に陥ることは想像に難くない。
私は、コードの美しさはその環境に依存すると考えている。
速度が求められる現場であれば、高速であることが美しさ
の最低条件であり、それを邪魔する一般的なベストプラクティスはむしろ汚さ
を加速させると考えている。
また、記述領域が圧倒的に少ないなど、リソースが限りなく少ない場合は、可能な限り圧縮されたコード
が、美しいだろう。
これは、世間一般のコーダーにおける美しさではないかもしれないが、そのコードが用いられる文脈によって、求められる要件は異なる。またそれを満たすことの手助けとならずに、却って邪魔となってしまう美しさ
は蛇足的である。
なぜ我々は、美しさを求めるのか、、?
それは、開発・運用・保守をより容易に行うことである。
そもそもの要件を満たさなくする美しさ
など悪
でしかないだろう。
つまり、美しさ
とは開発現場や要件に依存するため、一般的なベストプラクティス
が必ずしも正しいわけではなく、正しく文脈を理解して利用しなければ、むしろ汚いコードとなると考えている。
もし、あなたが求めている美しさがない状態の現場であるのであれば、なぜその現場ではそのようなコードとなっているのかをきちんと知る必要がある。
スープに小さな虫が入ると汚い
と感じるように、あなたの美しいコードは汚い
コードとほかの人に思われている可能性がある。
美しいコードの実践方法
ここまで、美しいコードの実践は難しいといったことをつらつらと考えてきた。
実際に実践は難しいのである。なぜなら、美しいコードはあまりにも目に見ないのである。
私にとっての美しいコード
は、あまりにも何もわからない状態である。
その原因は、良い
・悪い
、美しい
・汚い
といった言葉が多義的な言葉であることはもちろん、美しい
コードの全体像を見ることがうまくできないことが原因であろう。
群盲象を評す
出典(Wiki)
という慣用句があるが、現状はまさにそのような状況であるだろう。
全体像が見えないものに、全体像を考えた美しい
コードを実践することは不可能に近い。
また、逆に巨大な美しい
コードがある場(既存の美しい
とされおすすめされるようなOSSなどであるが)、抽象化などが適切に行われているため、むしろ初心者にとっては理解が難しい状況となってしまっているのだ。
そこで次からハンズオン的に、悪いコードをリファクタリングしながらどうしたらいいかを簡単に示したいと思う。
しかし、その前に私がコードレビューのタイミングでコードの記法で気にする部分をまとめたいと思う。
今回の文脈における美しい
コードとは
今回の文脈では、基本的に私が関わってきた開発が、小規模のWeb系
の開発であるため、速度や圧縮率などではなく、読みやすい
・変更が容易である
・依存が少ない
≒美しい
コードとしたい。
私がコードを読むときに重視していることは、
- main、もしくはそれに準ずる関数にIF文やloopなどが存在しないこと
- class, function, method, fileなどの命名は目的で作られること
- ビジネスロジック(コアドメイン)の中に、ロジックがまとまっていること
- Infrastracture層やApplication層など、ビジネスロジック以外が適切に分離、抽象化されていること
である。
それぞれに対して、個別にみていこうと思う
main、もしくはそれに準ずる関数にIF文やloopなどが存在しないこと
簡単に言うと、mainでは上から下まで簡単に読めるようにするべきであるということである
下手に、mainの中に分岐や繰り返しが存在すると、どこで何をしているのか、またその意図が読めなくなってしまうためである
可能な限り処理には名前をつけて、mainは呼び出すだけにすべきである
int main() {
int x = 5;
if (x > 0) {
for (int i = 0; i < x; i++) {
cout << "Hello, World!" << endl;
}
} else {
cout << "x is not positive." << endl;
}
return 0;
}
よく見るコードである。
この程度であれば問題ないが、今の段階でも、何を行っているかを想像するにはコードを読まなければならない。
コードの内容は読まずとも想像が可能であるべきである。
void printHelloXTimes(int count) {
for (int i = 0; i < count; i++) {
cout << "Hello, World!" << endl;
}
}
void notifyXIsNotPositive() {
cout << "x is not positive." << endl;
}
void printHelloOrNotifyBasedOnX(int x) {
if (x > 0) {
printHelloXTimes(x);
} else {
notifyXIsNotPositive();
}
}
int main() {
const int HELLO_WORLD_PRINT_COUNT = 5;
printHelloOrNotifyBasedOnX(HELLO_WORLD_PRINT_COUNT);
return 0;
}
プライベートメソッドは最悪のところ、多少雑でもよい が、ことPublicメソッドに関しては他人が利用するのである。今回はmainをPublic他をPrivateとして考えたときに、何をするのか一目瞭然の形にするのが良いだろう。
関数は増えたが、ネストも浅くなり、関数名だけで処理を想像できるようにしたため、わざわざ低レイヤーを除く必要がない。
このような抽象化が適切に行われているか
は、コードの美しさにおいて重要なことである。
特に、名前は重要である。この命名は目的に沿った名前を考えてつけるのが良いだろう
機能依存の命名では、そのコードが持つ文脈が読み取れず、誤った使い方をする可能性が高まるうえ、目的を確認するために、深い関数まで確認しに行かなければならない。
それはつまり、抽象化の失敗を意味しており、main関数において分岐や繰り返しをなくした目的が失われることを意味する。
class, function, method, fileなどの命名は目的で作られること
先ほどの項でほとんど説明してしまったが、目的に沿った名前を付けることで、抽象化が正しく行われやすい
const int X = 5;
const int Y = 10;
void doSomething(int count, bool flag) {
for (int i = 0; i < count; i++) {
if (flag) {
cout << "Hello, World!" << endl;
} else {
cout << "Goodbye, World!" << endl;
}
}
}
bool checkCondition(int count) {
return count > Y;
}
void processInput(int count) {
bool flag = checkCondition(count);
if (count > 0) {
doSomething(count, flag);
} else {
cout << "Count is not positive." << endl;
}
}
int main() {
processInput(X);
return 0;
}
ここまで極端なことは少ないが、processInputに近い内容はよく見る。この時なにも情報を得ることができないことが問題である。入れる処理
とは、何を?どのように?なんの目的で?と結局疑問が生まれるのである。
checkConditionもよく見る
。一見悪くないように見えるが、何の、どのような状態をチェックしているかが分からないのである。これも結局コードの文脈を確認しなければならない。
doSomethingこれは言うまでもなく致命的である。何かをする以外の情報がないのである。
私は、コードは限りなく読みたくないため、このコードは最悪である。
const int DEFAULT_GREETING_REPEAT_COUNT = 5;
const int SPECIAL_GREETING_THRESHOLD = 10;
void printGreeting(int count, bool useSpecialGreeting) {
for (int i = 0; i < count; i++) {
if (useSpecialGreeting) {
cout << "Hello, World!" << endl;
} else {
cout << "Goodbye, World!" << endl;
}
}
}
bool shouldUseSpecialGreeting(int count) {
return count > SPECIAL_GREETING_THRESHOLD;
}
void printGreetingBasedOnCount(int count) {
if (count > 0) {
bool useSpecialGreeting = shouldUseSpecialGreeting(count);
printGreeting(count, useSpecialGreeting);
} else {
cout << "Greeting repeat count must be positive." << endl;
}
}
int main() {
printGreetingBasedOnCount(DEFAULT_GREETING_REPEAT_COUNT);
return 0;
}
少し名前が長く、冗長的であるものの、少なくとも前よりは情報を理解することができる。
このコードにも改善の余地はあるが、少なくとも前よりはましであろう。
数字の意味や関数が何をするかが分かるのである。私はこのように読めるコードは美しいコードにつながる道であると考えている。
ビジネスロジック(コアドメイン)の中に、ロジックがまとまっていること
ビジネスロジック(コアドメイン)は、アプリケーションの中心的な機能や価値を提供する部分である。
このコアドメインは、ビジネスの専門家とソフトウェアエンジニアが密接に協力して開発すべき領域であるが、今回はドメイン理解はいったん置いておく。
簡潔にまとめるのであれば、単一責任目的駆動設計といえばよいのか、単一の責任を持つように目的ベースでクラスなどを作るようにすべきという考えである。
class Order {
public:
void calculateTotalPrice() {
// 注文の合計金額を計算するロジック
}
void saveToDatabase() {
// データベースに注文を保存するロジック
}
void sendConfirmationEmail() {
// 注文確認メールを送信するロジック
}
};
この例では、Order
クラスにビジネスロジック(calculateTotalPrice
)と技術的な実装の詳細(saveToDatabase
とsendConfirmationEmail
)が混在している。
これにより、クラスの責任が不明確になり、コードの理解と保守が難しくなる。
class Order {
public:
void calculateTotalPrice() {
// 注文の合計金額を計算するロジック
}
};
class OrderRepository {
public:
void save(Order order) {
// データベースに注文を保存するロジック
}
};
class OrderNotificationService {
public:
void sendConfirmationEmail(Order order) {
// 注文確認メールを送信するロジック
}
};
この例では、ビジネスロジック(calculateTotalPrice
)をOrder
クラスに集中させ、技術的な実装の詳細(save
とsendConfirmationEmail
)を別のクラス(OrderRepository
とOrderNotificationService
)に分離している。これにより、各クラスの責任が明確になり、コードの理解と保守が容易になる。
テストが書きやすい、ドメインエキスパートの援助を受けやすくなる、ドメイン知識が深まる...etcなどメリットは多数あるが、美しいコード
においては、単一の責任において、目的に沿った名前を付けて呼び出すことが重要である。
上記の本では完全コンストラクタなど、より美しいコードに近づける実践内容が存在しているため、ぜひ読んでみることをお勧めする
Infrastracture層やApplication層など、ビジネスロジック以外が適切に分離、抽象化されていること
先ほどまでと同様である。
それぞれが持つ目的に沿った場所にファイルなどをおき、抽象化を行うのである。その手法には様々あるが、インターフェースをかませることで、実際の呼び出しを抽象化することが可能である。
簡単にまとめる
簡単にまとめると、私はコードの美しさ
における要素としては、
- 疎結合・低依存であること
- 目的ベースの分かりやすい名前であること
- 単一の責任であること
この3点に集約されると考えている
そしてこれを実現することは意外とむずかしい。
しかしながら、基準を設けてその閾値を超えた場合に、関数の分化などを行うことを心がけることである程度は問題を解決することができる。
次にもう少し実践に近い、悪いコードをリファクタリングしながら読みやすいコードを作成していこうと思う。
実践に近い形でリファクタリング
実際に挙動確認はしていないので、動かない可能性が高い
極悪なコードを作成する
ネストが深く、flag引数によって結果が異なるような特に悪いコードである。
// 悪しき風習が詰まったmain関数
function main(csvFilePath1: string, csvFilePath2: string, jsonFilePath: string, flag1: boolean, flag2: boolean, flag3: boolean, maxAge: number, minAge: number, magicNumber: number) {
let data1: any[] = [], data2: any[] = [], mergedData: any[] = [], processedData: any[] = [];
if (flag1) {
const csv1 = readCsvFile(csvFilePath1);
const lines1 = csv1.split('\n');
for (let i = 1; i < lines1.length; i++) {
const line1 = lines1[i];
const [id, name, age, email] = line1.split(',');
const user = {
id: parseInt(id),
name,
age: parseInt(age),
email
};
data1.push(user);
}
}
if (flag2) {
const csv2 = readCsvFile(csvFilePath2);
const lines2 = csv2.split('\n');
for (let j = 1; j < lines2.length; j++) {
const line2 = lines2[j];
const [userId, phone, address, city, state, zip, country] = line2.split(',');
const user = {
id: parseInt(userId),
phone,
address: {
street: address,
city,
state,
zip,
country
}
};
data2.push(user);
}
}
if (flag1 && flag2) {
for (const user1 of data1) {
for (const user2 of data2) {
if (user1.id === user2.id) {
const mergedUser = { ...user1, ...user2 };
mergedData.push(mergedUser);
break;
}
}
}
} else {
mergedData = [...data1, ...data2];
}
for (const user of mergedData) {
if (user.age > minAge && user.age < maxAge) {
if (user.email && user.email.endsWith('@example.com')) {
if (user.phone && user.phone.startsWith('555')) {
if (user.address && user.address.country === 'USA') {
if (user.address.state === 'CA' || user.address.state === 'NY') {
if (user.address.city === 'Los Angeles' || user.address.city === 'New York') {
if (user.name && user.name.startsWith('J')) {
if (user.id % magicNumber === 0) {
processedData.push(user);
}
}
}
}
}
}
}
}
}
if (flag3) {
processedData.sort((a, b) => b.age - a.age);
} else {
processedData.sort((a, b) => a.name.localeCompare(b.name));
}
const jsonData = JSON.stringify(processedData, null, 2);
writeJsonFile(jsonFilePath, jsonData);
}
この関数には、for も if も含まれており、2以上のネストが含まれている。
また、よくわからないフラグが立っている。
他にもわかりづらい要素が多数あるが、ひとまずよくないコードであることが分かれば問題ないだろう。
それでは、この関数に適切だと思う名前を付けることにしたい。
どのような名前になっただろうか。
恐らく、すべての処理内容を表すことはできないだろう。
それを行おうとすると、processAndFilterUserDataFromMultipleCsvFilesByAgeEmailPhoneLocationAndIdThenSortAndSaveToJsonFileBasedOnFlags
など、膨大な名称となることが想像できる。
動詞が複数出てくる場合、関数の責任は単一の状態になっていない可能性が高い
命名と単一の責任は深い関係である。
素直にすべての処理を含めた命名を行うときに、その関数やメソッドに対してすんなりと目的を示しながら名づけを行うことができるだろうか。
これが素直に行える場合は、単一の責任になっている可能性が高い。
しかし、命名が素直に行えない場合は、別の責任を伴っていると仮定して分割すべきである。
関数を分割する
巨大な関数はそれだけで読みづらく、わかりにくくなりやすい。
次に関数を分割して、一つの関数が5行以内になるようにしてみよう。
function main(csvFilePath1: string, csvFilePath2: string, jsonFilePath: string, flag1: boolean, flag2: boolean, flag3: boolean, maxAge: number, minAge: number, magicNumber: number) {
let data1: any[] = [], data2: any[] = [], mergedData: any[] = [], processedData: any[] = [];
processData1(csvFilePath1, csvFilePath2, flag1, flag2, data1, data2, mergedData, minAge, maxAge, magicNumber, processedData, flag3, jsonFilePath);
}
function processData1(csvFilePath1: string, csvFilePath2: string, flag1: boolean, flag2: boolean, data1: any[], data2: any[], mergedData: any[], minAge: number, maxAge: number, magicNumber: number, processedData: any[], flag3: boolean, jsonFilePath: string) {
if (flag1) {
const csv = readCsvFile(csvFilePath1);
const lines = csv.split('\n');
processLines(lines, data1);
processData2(csvFilePath2, flag2, data1, data2, mergedData, minAge, maxAge, magicNumber, processedData, flag3, jsonFilePath);
}
}
function processData2(csvFilePath: string, flag: boolean, data1: any[], data2: any[], mergedData: any[], minAge: number, maxAge: number, magicNumber: number, processedData: any[], flag3: boolean, jsonFilePath: string) {
if (flag) {
const csv = readCsvFile(csvFilePath);
const lines = csv.split('\n');
processLines2(lines, data2);
mergeData(data1, data2, mergedData, minAge, maxAge, magicNumber, processedData, flag3, jsonFilePath);
}
}
function processLines(lines: string[], data: any[]) {
for (let i = 1; i < lines.length; i++) {
const line = lines[i];
const [id, name, age, email] = line.split(',');
const user = {
id: parseInt(id),
name,
age: parseInt(age),
email
};
data.push(user);
}
}
function processLines2(lines: string[], data: any[]) {
for (let j = 1; j < lines.length; j++) {
const line = lines[j];
const [userId, phone, address, city, state, zip, country] = line.split(',');
const user = {
id: parseInt(userId),
phone,
address: {
street: address,
city,
state,
zip,
country
}
};
data.push(user);
}
}
function mergeData(data1: any[], data2: any[], mergedData: any[], minAge: number, maxAge: number, magicNumber: number, processedData: any[], flag3: boolean, jsonFilePath: string) {
if (data1.length > 0 && data2.length > 0) {
mergeDataByUserId(data1, data2, mergedData);
} else {
mergedData.push(...data1, ...data2);
}
filterAndProcessData(mergedData, minAge, maxAge, magicNumber, processedData, flag3, jsonFilePath);
}
function mergeDataByUserId(data1: any[], data2: any[], mergedData: any[]) {
for (const user1 of data1) {
for (const user2 of data2) {
if (user1.id === user2.id) {
const mergedUser = { ...user1, ...user2 };
mergedData.push(mergedUser);
break;
}
}
}
}
function filterAndProcessData(mergedData: any[], minAge: number, maxAge: number, magicNumber: number, processedData: any[], flag3: boolean, jsonFilePath: string) {
for (const user of mergedData) {
if (isUserValid(user, minAge, maxAge, magicNumber)) {
processedData.push(user);
}
}
sortData(processedData, flag3, jsonFilePath);
}
function isUserValid(user: any, minAge: number, maxAge: number, magicNumber: number): boolean {
return user.age > minAge && user.age < maxAge &&
user.email && user.email.endsWith('@example.com') &&
user.phone && user.phone.startsWith('555') &&
user.address && user.address.country === 'USA' &&
(user.address.state === 'CA' || user.address.state === 'NY') &&
(user.address.city === 'Los Angeles' || user.address.city === 'New York') &&
user.name && user.name.startsWith('J') &&
user.id % magicNumber === 0;
}
function sortData(processedData: any[], flag: boolean, jsonFilePath: string) {
if (flag) {
processedData.sort((a, b) => b.age - a.age);
} else {
processedData.sort((a, b) => a.name.localeCompare(b.name));
}
saveDataToJsonFile(processedData, jsonFilePath);
}
function saveDataToJsonFile(processedData: any[], jsonFilePath: string) {
const jsonData = JSON.stringify(processedData, null, 2);
writeJsonFile(jsonFilePath, jsonData);
}
これらの関数が何も問題を解決できていないことが分かるだろうか。
最初の関数から、最後の関数まで、ただ関数を呼び出し続けているのである。
これでは、先ほどまでの巨大な関数と変わらない。
確かに、巨大な関数を分割することは読みやすいコード
を実現するうえで重要な一歩である
しかし、そこに強烈な依存がある場合に、コードを読むことは相当に難しくなる。
なぜなら、コードを最後まで読む必要があるからである。
コードは可能な限り抽象化され、最後まで読む必要がないほうが都合が良い
これではむしろ、もとの巨大な関数のほうが一連の流れを確認できるだけましかもしれない。
依存を解かない関数の分割はむしろより悪い結果を招く可能性がある
依存を解いて関数化する
それでは次に依存をなくすように関数化を行う。
// リファクタリングされたmain関数
function main(csvFilePath1: string, csvFilePath2: string, jsonFilePath: string, flag1: boolean, flag2: boolean, flag3: boolean, maxAge: number, minAge: number, magicNumber: number) {
console.log('処理を開始します');
const data1 = loadUserDataFromCsv(csvFilePath1, flag1);
const data2 = loadUserDetailsFromCsv(csvFilePath2, flag2);
const mergedData = combineUserData(data1, data2, flag1, flag2);
const processedData = filterAndSortUserData(mergedData, minAge, maxAge, magicNumber, flag3);
saveDataToJsonFile(processedData, jsonFilePath);
console.log('処理が完了しました');
}
function loadUserDataFromCsv(csvFilePath: string, shouldLoad: boolean): any[] {
if (!shouldLoad) return [];
const csv = readCsvFile(csvFilePath);
const lines = csv.split('\n');
const userData = lines.slice(1).map(parseUserData);
console.log('ユーザーデータをロードしました');
return userData;
}
function parseUserData(line: string): any {
const [id, name, age, email] = line.split(',');
return { id: parseInt(id), name, age: parseInt(age), email };
}
function loadUserDetailsFromCsv(csvFilePath: string, shouldLoad: boolean): any[] {
if (!shouldLoad) return [];
const csv = readCsvFile(csvFilePath);
const lines = csv.split('\n');
const userDetails = lines.slice(1).map(parseUserDetails);
console.log('ユーザー詳細をロードしました');
return userDetails;
}
function parseUserDetails(line: string): any {
const [userId, phone, address, city, state, zip, country] = line.split(',');
return { id: parseInt(userId), phone, address: { street: address, city, state, zip, country } };
}
function combineUserData(userData: any[], userDetails: any[], shouldCombine: boolean, shouldMerge: boolean): any[] {
if (!shouldCombine || !shouldMerge) return [...userData, ...userDetails];
const mergedData = userData.map(user => ({ ...user, ...userDetails.find(detail => detail.id === user.id) }));
console.log('ユーザーデータを結合しました');
return mergedData;
}
function filterAndSortUserData(userData: any[], minAge: number, maxAge: number, magicNumber: number, shouldSort: boolean): any[] {
const filteredData = userData.filter(isValidUser(minAge, maxAge, magicNumber));
const sortedData = shouldSort ? filteredData.sort(byAgeDescending) : filteredData.sort(byNameAscending);
console.log('ユーザーデータをフィルタリングし、ソートしました');
return sortedData;
}
function isValidUser(minAge: number, maxAge: number, magicNumber: number): (user: any) => boolean {
return user =>
user.age > minAge && user.age < maxAge &&
user.email?.endsWith('@example.com') &&
user.phone?.startsWith('555') &&
user.address?.country === 'USA' &&
(user.address?.state === 'CA' || user.address?.state === 'NY') &&
(user.address?.city === 'Los Angeles' || user.address?.city === 'New York') &&
user.name?.startsWith('J') &&
user.id % magicNumber === 0;
}
function byAgeDescending(a: any, b: any): number {
return b.age - a.age;
}
function byNameAscending(a: any, b: any): number {
return a.name.localeCompare(b.name);
}
function saveDataToJsonFile(data: any[], jsonFilePath: string) {
const jsonData = JSON.stringify(data, null, 2);
writeJsonFile(jsonFilePath, jsonData);
console.log('データをJSONファイルに保存しました');
}
先ほどまでよりも良いコードとなったことが分かる。
最初に呼び出されるmain関数を読むだけで、処理の流れが分かり、どのようなことを行っているかが理解できるからである
内部を読む必要がないのである。
こうしたときに、処理の途中に新しい機能を組み込むことが容易になったことが分かるだろう
例えば、表示処理を加えてみる
function main(csvFilePath1: string, csvFilePath2: string, jsonFilePath: string, flag1: boolean, flag2: boolean, flag3: boolean, maxAge: number, minAge: number, magicNumber: number) {
console.log('処理を開始します');
const data1 = loadUserDataFromCsv(csvFilePath1, flag1);
console.log('loadUserDataFromCsv',data1)
const data2 = loadUserDetailsFromCsv(csvFilePath2, flag2);
console.log('loadUserDetailsFromCsv',data2)
const mergedData = combineUserData(data1, data2, flag1, flag2);
const processedData = filterAndSortUserData(mergedData, minAge, maxAge, magicNumber, flag3);
saveDataToJsonFile(processedData, jsonFilePath);
console.log('処理が完了しました');
}
もとの関数に一切の手を加える必要がない。
これが抽象化の一つのメリットである
また、全体の流れが上からまっすぐと読むことが可能であるため、読みやすいコードとなっている。
しかし、これでもなんだか読みづらく感じないだろうか
なぜだろうか?
それは関数が目的ごとにまとまっているわけではないためである。
次は目的ごとに、クラスにまとめたい。
目的別でクラスにまとめる
src/
domain/
user.ts
application/
user-service.ts
user-repository.interface.ts
infrastructure/
csv-user-repository.ts
index.ts
export class User {
constructor(
public id: number,
public name: string,
public age: number,
public email: string,
public phone?: string,
public address?: {
street: string;
city: string;
state: string;
zip: string;
country: string;
}
) {}
}
本来は、引数の確認などを行うべきであるが、今回に関しては概要であるので、データクラスとなってしまっているが黙認する
import { User } from '../domain/user';
export interface UserRepository {
findAll(): Promise<User[]>;
save(users: User[]): Promise<void>;
}
import { User } from '../domain/user';
import { UserRepository } from './user-repository.interface';
export class UserService {
constructor(
private userRepository: UserRepository,
private minAge: number,
private maxAge: number,
private magicNumber: number
) {}
async getAllUsers(): Promise<User[]> {
const users = await this.userRepository.findAll();
const filteredUsers = this.filterUsers(users);
const sortedUsers = this.sortUsers(filteredUsers);
return sortedUsers;
}
async saveUsers(users: User[]): Promise<void> {
await this.userRepository.save(users);
}
private filterUsers(users: User[]): User[] {
const filteredUsers = users.filter(user =>
user.age > this.minAge &&
user.age < this.maxAge &&
user.email.endsWith('@example.com') &&
user.phone?.startsWith('555') &&
user.address?.country === 'USA' &&
(user.address?.state === 'CA' || user.address?.state === 'NY') &&
(user.address?.city === 'Los Angeles' || user.address?.city === 'New York') &&
user.name.startsWith('J') &&
user.id % this.magicNumber === 0
);
console.log('ユーザーデータをフィルタリングしました');
return filteredUsers;
}
private sortUsers(users: User[]): User[] {
const sortedUsers = users.sort((a, b) => a.name.localeCompare(b.name));
console.log('ユーザーデータをソートしました');
return sortedUsers;
}
}
import { User } from '../domain/user';
import { UserRepository } from '../application/user-repository.interface';
import { readCsvFile, writeJsonFile } from './file-utils';
export class CsvUserRepository implements UserRepository {
constructor(
private userDataFilePath: string,
private userDetailsFilePath: string
) {}
async findAll(): Promise<User[]> {
const userData = await this.loadUserData();
const userDetails = await this.loadUserDetails();
const users = this.mergeUserData(userData, userDetails);
return users;
}
async save(users: User[]): Promise<void> {
const jsonData = JSON.stringify(users, null, 2);
writeJsonFile('output.json', jsonData);
console.log('データをJSONファイルに保存しました');
}
private async loadUserData(): Promise<any[]> {
const csv = await readCsvFile(this.userDataFilePath);
const lines = csv.split('\n');
const userData = lines.slice(1).map(line => {
const [id, name, age, email] = line.split(',');
return { id: parseInt(id), name, age: parseInt(age), email };
});
console.log('ユーザーデータをロードしました');
return userData;
}
private async loadUserDetails(): Promise<any[]> {
const csv = await readCsvFile(this.userDetailsFilePath);
const lines = csv.split('\n');
const userDetails = lines.slice(1).map(line => {
const [userId, phone, address, city, state, zip, country] = line.split(',');
return { id: parseInt(userId), phone, address: { street: address, city, state, zip, country } };
});
console.log('ユーザー詳細をロードしました');
return userDetails;
}
private mergeUserData(userData: any[], userDetails: any[]): User[] {
const users = userData.map(data => {
const details = userDetails.find(detail => detail.id === data.id);
return new User(
data.id,
data.name,
data.age,
data.email,
details?.phone,
details?.address
);
});
console.log('ユーザーデータを結合しました');
return users;
}
}
import { UserService } from './application/user-service';
import { CsvUserRepository } from './infrastructure/csv-user-repository';
async function main() {
const userRepository = new CsvUserRepository('users.csv', 'user_details.csv');
const userService = new UserService(userRepository, 18, 60, 3);
console.log('処理を開始します');
const users = await userService.getAllUsers();
await userService.saveUsers(users);
console.log('処理が完了しました');
}
main();
このように、データの保存と処理内容などを分割することで、クラスにまとまりが生じ、何をしているかが一気にわかりやすくなる。
また、user-serviceでは
async getAllUsers(): Promise<User[]> {
const users = await this.userRepository.findAll();
const filteredUsers = this.filterUsers(users);
const sortedUsers = this.sortUsers(filteredUsers);
return sortedUsers;
}
このようにインターフェースに依存することで、実装に依存せずに抽象化している。
今回のように簡易的にファイルに保存している現状から、要件が変わりDBに保存する場合には、新しくDBに保存するようのレポジトリを作成し、mainで注入している依存内容をDBの内容に置き換えればよいのである。
serviceクラスの内容を全く気にせずに変更が可能な点を考えるとそのメリットが伝わるだろう。
全体のまとめ
今回は最終的に、読みやすい
・変更が容易である
・依存が少ない
≒美しい
と美しさを定義して変更した。
初めはできるところから、命名に気を付け、命名が素直にできない場合は実装を疑うようにするところから、美しい
コードは始まると思う。
また、低依存
なコードの実現には、テストをお勧めする。
テストを行うには、低依存のコードであることが前提条件となる。
密結合であると、DBのテストやAPIのテストを行うと実際のコードが走ってしまう問題が発生し、テストの実現が難しくなる。
テストを書きながら、テストが書きやすいコードであることを意識しながら記述することで、依存は圧倒的に少なくなる。
改めて、これは現場に出ていない小僧が書いた戯言であるため、実際には大幅に違う可能性もある。しかし、ある一定の効果も見込めると思うので、現場の文脈を調整しながら実践の一助となれば幸いである。