他人が作ったコードをリファクタリングしてみる part1 @uni928

今回は他人が作った下記のコードを
「メソッド数は増やしたくない」
「変数名を変える作業をしたくない」
という条件でリファクタリングします。

リファクタリング前のコード

function createOutPng() {
	let ulx=outputUl%(row*2+1);
	let uly=Math.floor(outputUl/(col*2+1));
	let drx=outputDr%(row*2+1);
	let dry=Math.floor(outputDr/(col*2+1));

	let tulx=areaX+(boxSize/2)*ulx;
	let tuly=areaY+(boxSize/2)*uly;
	let tdrx=areaX+(boxSize/2)*drx;
	let tdry=areaY+(boxSize/2)*dry;

	let imageData=document.getElementById("outputcanvas").getContext("2d").getImageData(tulx, tuly, (tdrx-tulx), (tdry-tuly));

	let canvas=document.createElement("canvas");
	canvas.width=(tdrx-tulx);
	canvas.height=(tdry-tuly);
	canvas.getContext("2d").putImageData(imageData, 0, 0);
	let png=canvas.toDataURL("image/png");//console.log(png);

	let newImg=document.createElement("img");
	newImg.src=png;
	newImg.style.position="absolute";
	newImg.style.zIndex="14906";
	newImg.style.left="200px";
	newImg.style.top="120px";
	newImg.id="outputimg";
}

コードを見ただけでは
分からない箇所がほどんどだと思われますが
皆さんならばどのようにリファクタリングしますか？

コードの内容が分からないなりにで良いので
よろしければ一度考えてみて下さい。

考えて頂けましたか？
私は下記のようにリファクタリングしました。

このコードはリファクタリングが難しいため
せめてスコープが分かりにくい点を
修正してみたいと思います。

リファクタリング後のコード

function createOutPng() {
	let ulx=outputUl%(row*2+1);
	let uly=Math.floor(outputUl/(col*2+1));
	let drx=outputDr%(row*2+1);
	let dry=Math.floor(outputDr/(col*2+1));

	let tulx=areaX+(boxSize/2)*ulx;
	let tuly=areaY+(boxSize/2)*uly;
	let tdrx=areaX+(boxSize/2)*drx;
	let tdry=areaY+(boxSize/2)*dry;

    ulx = uly = drx = dry = undefined; //修正点1

	let imageData=document.getElementById("outputcanvas").getContext("2d").getImageData(tulx, tuly, (tdrx-tulx), (tdry-tuly));

	let canvas=document.createElement("canvas");
	canvas.width=(tdrx-tulx);
	canvas.height=(tdry-tuly);
	canvas.getContext("2d").putImageData(imageData, 0, 0);
	let png=canvas.toDataURL("image/png");//console.log(png);

	tulx = tuly = tdrx = tdry = undefined; //修正点2

	let newImg=document.createElement("img");
	newImg.src=png;
	newImg.style.position="absolute";
	newImg.style.zIndex="14906";
	newImg.style.left="200px";
	newImg.style.top="120px";
	newImg.id="outputimg";
}

修正点1

ulx = uly = drx = dry = undefined; //修正点1

と

修正点2

tulx = tuly = tdrx = tdry = undefined; //修正点2

を追記しました。

このリファクタリングの狙いについて
紹介したいと思います。

---

修正点 1 の狙い

修正点1

ulx = uly = drx = dry = undefined; //修正点1

を追記した狙いは
これらの変数が以降で使わない事が
すぐに分かるようにするためです。

ulx と tulx が混在しているため
この行を書かないと

canvas.width=(tdrx-tulx);
canvas.height=(tdry-tuly);

の箇所を読む際に
少し混乱してしまいます。
混乱しないために undefined に戻します。

---

修正点 2 の狙い

修正点2

tulx = tuly = tdrx = tdry = undefined; //修正点2

を追記した狙いは
修正点 1 とは異なり
「ここまで使用する」事が
一目で分かるようにするためです。

getImageData で使った後は
これらの変数はもう使わないように
見えてしまうため、
後ろの方で undefined に設定しているのを見て
「まだ使うんだ」という気づきに
繋がる可能性があります。

---

以上特定のコードをリファクタリングする
第 1 弾を公開しました。

スコープを分かりやすくするため
undefined に設定する手法を紹介しました。

undefined に設定することで
「この変数はもう使わない」
「この変数はここまで使う」
というのが一目で分かるという結論でした。

皆さんは使わなくなった変数は
undefined に戻していますか？
よろしければコメント下さい。

皆さんが他人のコードを
リファクタリングする際の
参考になれますように。

閲覧ありがとうございました。