手順を差し替えるリファクタリング

著者:杉浦

手順を差し替えるリファクタリング

 コードを整理するための手法であるリファクタリング手法には名前の置き換えや抽出などがあります。しかし時折いささか複雑な手順を実行しているコードに出会うことがあります。その様なコードはどこをどう切り取っても名前を置き換えても複雑なままで事故の元になります。そういった時は、入力と出力が同じになる様に注意して、別の単純なコードに置き換えた方が簡単に整理できます。
 例えば、次です。

e = document.querySelectorAll('a, span, div');
f = false;
e.forEach(function(e){
  if(e.innerText==='hoge'){
    f=true;
  }
});
if(f){
  console.log('hoge is exist');
} else {
  console.log('hoge is not exist');
}

 即興の例なのでまあまあシンプルです。実際はもっと長大なコードであったりします。このコードを置き換えてみます。置き換えで重要なのは入口と出口を壊さないことです。この例における入り口とは

e = document.querySelectorAll('a, span, div');

であり、出口とは

if(f){
  console.log('hoge is exist');
} else {
  console.log('hoge is not exist');
}

です。入口と出口はリファクタリング対象の部分の目的によって変わります。もし、変数fがこのコードの後に再利用されるならば、変数fも出口の一部扱いです。

 リファクタリングの際には入口と出口を変えていないことを確認しながら作業します。

const elements = document.querySelectorAll('a, span, div');// コピペ

// 要素リストelementsの中にinnerText==='hoge'となる要素が存在するかで分岐

const msg = exist_hoge ? 'hoge is exist' : 'hoge is not exist';
console.log(msg);

 ここでは目的が定まっているため大雑把に消しました。他人のコードや過去のコードをいじる際には目的が正しいかを確認する必要があります。テストが用意されているならばそれに従いましょう。
 入口出口が安全に保護されているので内部を好きに弄れます。最終的に次のコードにしました。

const elements = document.querySelectorAll('a, span, div');// コピペ

const exist_hoge = Array.from(elements).map(element=>element.innerText).indexOf('hoge') !== -1;

const msg = exist_hoge ? 'hoge is exist' : 'hoge is not exist';
console.log(msg);

 ワンライナーで済むようになりました。

  • この記事いいね! (0)

著者について

杉浦 administrator