自動生成されたHTMLから社員情報を引っこ抜くスクリプトを作った時に考えたことを書き出してみる。ブラウザのDevToolsを使った簡易ツールを書いていて、頭に浮かんだことだ。データやコードは架空の内容なので実在しない。
大まかに最初に目に入ってきたのはこんな内容だった。機械生成されているせいかフォーマットがガタガタ気味だ。
<span class="Content">
<span id="undefined">SHAIN.XLS page1</span>
</span>
<span class="Content">
<br>
<span id="undefined">社員リスト</span>
<span id="undefined"></span>
</span>
<span class="Content">
<br>
<span id="hoge1">00001</span>
<span id="fuga1">：山菱喜一</span>
</span>
<span class="Content">
<br>
<span id="hoge2">00002</span>
<span id="fuga2">：鈴木与一</span>
</span>
<span class="Content">
<br>
<span id="hoge3">00011</span>
<span id="fuga3">：茨城妙子</span>
</span>
<span class="Content">
<span id="undefined">SHAIN.XLS</span>
<span id="undefined"> page2</span>
</span>
<span class="Content">
<br>
<span id="hoge4">00012</span>
<span id="fuga4">：奈良楓</span>
</span>
...
ひとまず普段は関数ベースで実装しているので関数ベースで実装する。書いていてgetEmployee(name, list)の仮引数にあるlistは冗長ではないかということに気づく。
const emp_list = document.getElementsByClassName('Content');
[...emp_list].reduce((acc, cur) => {
if (cur.id === 'undefined' || cur.children.length !== 3) {
return acc;
} else {
const id = cur.children[1].innerText;
const name = cur.children[2].innerText.replace(/^：/, '');
if (/^\d+$/.test(id)) {
acc.push({ id, name });
}
return acc;
}
}, []);
const getEmployee = (name, list) => {
const re = new RegExp(`.*${name}.*`);
const result = list.filter((emp) => re.test(emp));
console.log(result);
};
そしてクラスにしたほうが素直にならないかと考え、クラスにしてみた。getEmployee(name, list)はsearch(name)となり、冗長性が減ったほか、抽象的な表現で分かりやすくなった気がする。
class EmployeeSearch {
constructor() {
const emp_list = document.getElementsByClassName('Content');
this.list = [...emp_list].reduce((acc, cur) => {
if (cur.id === 'undefined' || cur.children.length !== 3) {
return acc;
} else {
const id = cur.children[1].innerText;
const name = cur.children[2].innerText.replace(/^：/, '');
if (/^\d+$/.test(id)) {
acc.push({ id, name });
}
return acc;
}
}, []);
}
search(name) {
const re = new RegExp(`.*${name}.*`);
const result = this.list.filter((emp) => re.test(emp.name));
console.log(result);
}
}
動作確認をしていく中で表示と名前が分かれたパターンを発見したので、これに合わせて直してゆく。
<span class="Content">
<br>
<span id="hoge2">00002</span>
<span id="fuga2">：飯田</span>
<span id="piyo2">太郎</span>
</span>
この対応をしようとすると分岐が必要だと考えたが、愚直にやってしまうとコンストラクタの中がごちゃつく上、テストもしづらいと考え、DOMをパースする処理を外部関数に切り出し、それを呼ぶことにした。
const getEmployeeFromList = (domList) => {
const id = domList[1].innerText;
const name1 = domList[2].innerText.replace(/^：/, '');
if (domList.length === 3) {
return {
id,
name: name1
}
} else {
const name2 = domList[3].innerText;
return {
id,
name: `${name1}${name2}`
}
}
};
class EmployeeSearch {
constructor() {
const emp_list = document.getElementsByClassName('Content');
this.list = [...emp_list].reduce((acc, cur) => {
if (cur.id === 'undefined' || cur.children.length < 3) {
return acc;
} else {
const { id, name } = getEmployeeFromList(cur.children);
if (/^\d+$/.test(id)) {
acc.push({ id, name });
}
return acc;
}
}, []);
}
search(name) {
const re = new RegExp(`.*${name}.*`);
const result = this.list.filter((emp) => re.test(emp.name));
console.log(result);
}
}
ブラウザのDevToolsにあるConsoleで書いたコードなので、現実問題テストコードも書かないし、そこまで凝る必要もないのが、よりよい設計にするにはコンストラクタでパースせず、事前にパース処理した結果をコンストラクタを通じて注入して処理するようにするとよいだろう。そうすると責務が分離され、テストがしやすく、コードもきれいになると考える。
これはまだ正直答えが見えていないが、インスタンスを引き回す場合はクラスのほうが都合がいい可能性がある気がしてきている。
関数でもできるにはできるが、毎回引数にインスタンス相当のものを渡すのは冗長である可能性がある。疎結合という文脈では関数のほうがより良い可能性もあるが、人間の認知負荷や、保守性との兼ね合いだと思う。
明確な損益分岐点はまだ見えていないし、今回のケースだと割とどっちでもいい部分もあるとは思う。
