悪いコード
検索ワードとソートを指定して検索する何かのシステムを考えます。
クエリを作成するクラスです。
ソートの順番に指定があり、例えばユーザーが「カテゴリー」でソートをした場合、第二ソートキーは「値段」の昇順、第三ソートキーは「更新日時」の降順です。
//以下はユーザーから受け取る
//検索ワード
public string Word { get; set;}
//ソートするアイテム
public string SortItem { get; set;}
//ソートする方向
public string SortDirection { get; set;}
// クエリを作成
public string create() {
// SortItemの値によって第二ソートキー、第三ソートキーを決める
string secondSort;
string secondSortDic;
string thirdSort;
string thirdSortDic;
switch(SortItem) {
case "category" :
// カテゴリーでソートした場合、第二ソートキーは値段の昇順、第三ソートキーは更新日時の降順
secondSort = "price";
secondSortDic = "Asc";
thirdSort = "update"
thirdSortDic = "Desc";
case "rank" :
secondSort = "orderNum";
・・・
}
// JSONのクエリを作成
return $@"{{
word: {Word},
sortItem: {SortItem},
sortDirection: {SortDirection}
secondSort: {secondSort},
secondSortDic: {secondSortDic},
thirdSort: {thirdSort},
thirdSortDic: {thirdSortDic},
}}";
当初の私のコードはこの通り、スイッチ文で分岐をさせていました。
めんどくさくて1つのcaseしか書いていませんが、実際はこれがあと4つありました。
良いコード
こちらが、私がミノ駆動本を読んだ後書いたコードです。
//コンストラクタを追加
CreateQuery (string sortItem, string sortDirection){
SortItem = sortItem;
SortDirection = sortDirection;
}
//検索ワード
public string Word { get; set;}
//ソートするアイテム
public string SortItem { get; set;}
//ソートする方向
public string SortDirection { get; set;}
// ソートクエリを作成するメソッドを用意する。
public List<CreateQuery> getSortQuery() {
switch(SortItem) {
case "category" :
// カテゴリーでソートした場合、第二ソートキーは値段の昇順、第三ソートキーは更新日時の降順
// 第二ソートキー、第三ソートキーごとに新しいインスタンスを作成する。
secondSort = new CreateQuery("price", "Asc");
thirdSort = new CreateQuery("update", "Desc");
// 第一ソートキーであるこれ自身とともに配列にして返す。
return new List<CreateQuery>{this, secondSort, thirdSort};
case "rank" :
secondSort = new CreateQuery("orderNum", "Asc");
・・・
}
}
// クエリを作成
public string create() {
var sortList = getSortQuery();
// JSONのクエリを作成
return $@"{{
word: {Word},
sortItem: {sortList[0].SortItem},
sortDirection: {sortList[0].SortDirection}
secondSort: {sortList[1].SortItem},
secondSortDic: {sortList[1].SortDirection},
thirdSort: {sortList[2].SortItem},
thirdSortDic: {sortList[2].SortDirection},
}}";
}
どうでしょうか。
良い、とまでは言えません。もう少しスマートな方法がありそうですが、前回のコードよりは改善されているのではないでしょうか。
インスタンスを戻り値にする、なにより、自分自身を返すインスタンスを戻り値とするというのは初心者である私にはなかなか思いつかない発想でした。
ミノ駆動本に出会っていなければ、このようなコードは書けなかったと思います。
ただ、本当にこの設計が合っているのかよく分からなかったのも事実です。
しかし、この後、追加で機能を実装するよう言われた際に、この設計が大きな効果を発揮することを実感しました。
それは、全てのソートの方向を反転させたクエリを作るメソッドを作るというものです。
// 現在のソートの方向を反転させた新しいインスタンスを返すメソッド
public CreateQuery ReverseDirection() {
string reverseDic = this.SortDirection == "Asc" ? "Desc" : "Asc";
return new CreateQuery(this.SortItem, reverseDic);
}
public string createReverse() {
// ソートキーを取得したあと、Selectの中でReverseDirection()を使用し全て反転
var sortList = getSortQuery().Select(x => x.ReverseDirection());
// JSONのクエリを作成
return $@"{{
word: {Word},
sortItem: {sortList[0].SortItem},
sortDirection: {sortList[0].SortDirection}
secondSort: {sortList[1].SortItem},
secondSortDic: {sortList[1].SortDirection},
thirdSort: {sortList[2].SortItem},
thirdSortDic: {sortList[2].SortDirection},
}}";
}
簡単に反転したクエリを出力するコードが記述できました。
見やすさはともかく、変更容易性という観点では良いコードになったのではないでしょうか。
記述してみて思ったこと
これまでの学習で一通り動くコードを作ることはできていましたが、可読性や変更容易性という点ではイマイチ納得できるコードを書けませんでした。
リーダブルコードを読んでも成長できず、もやもやした日々でした。
しかし、今回ミノ駆動本を読み、初めてこれが分かりやすいコードか!と思えました。
筆者のミノ駆動さんは「リファクタリング」という本に出会って人生が変わったと述べていましたが、私はこの本で人生が変わったと思います。
改めてこうして記事にしてみて、自分が書いたコードを振り返るとなんか見にくいなと感じてしまいますが、いずれは修正できるように努力していきたいです。