昨日は if の書き方で息抜きコーディングしたんだけど
今日もこんなツイートを見かけたので息抜きをすることにした。if が流行ってるのかな?
一番上の書き方が一番好きなんだけど一番下が好みの人がいて解せぬ。真ん中は万死に値すると思う。 pic.twitter.com/hxTGUdMhki
— ぐっちょむ (@gutchom) 2022年8月3日
元のコード
(関数名は変えちゃった)
export function type1(prevX, prevY, nextX, nextY) { const toLeft = prevX - nextX > 0; const toRight = prevX - nextX < 0; const toTop = prevY - nextY > 0; const toBottom = prevY - nextY < 0; return { horizontal: toLeft ? 'left' : toRight ? 'right' : 'neutral', vertical: toTop ? 'top' : toBottom ? 'bottom' : 'neutral', }; } export function type2(prevX, prevY, nextX, nextY) { return { horizontal: prevX - nextX > 0 ? 'left' : prevX - nextX < 0 ? 'right' : 'neutral', vertical: prevY - nextY > 0 ? 'top' : prevY - nextY < 0 ? 'bottom' : 'neutral', }; } export function type3(prevX, prevY, nextX, nextY) { let horizontal = 'neutral'; if (prevX - nextX > 0) { horizontal = 'left'; } if (prevX - nextX < 0) { horizontal = 'right'; } let vertical = 'neutral'; if (prevY - nextY > 0) { vertical = 'top'; } if (prevY - nextY < 0) { vertical = 'bottom'; } return { horizontal, vertical }; }
おもしろいね
感想
三項演算子はあんまり好きじゃなくて、Reactのコンポーネントぐらいでしか使わないけど、これくらいならどれでもいっかな。UTがあればあとで書き換えたくなった人が書き換えたらいいかなーくらいで。
— Mitsuyuki Shiiba (@bufferings) August 4, 2022
もし、こういうコードのプルリクエストが来たら、自分の好みはあるからそれは伝えるけど、「これが自分にとってのベストなんだー!」って理由を説明してくれるなら、別にどれでもいいかなという気持ち
で、自分の好みは?
僕だったら、こうかくかなぁ。昨日の記事に影響受けてる!
export function type3_3(prevX, prevY, nextX, nextY) { const horizontal = (() => { if (prevX > nextX) return 'left'; if (prevX < nextX) return 'right'; return 'neutral'; })(); const vertical = (() => { if (prevY > nextY) return 'top'; if (prevY < nextY) return 'bottom'; return 'neutral'; })(); return { horizontal, vertical }; }
メインのコードの話はここでおしまい
ところでUT
今日のお昼にブロッコリーさんの記事をみて、なるほどー。ってなったので、今日は Jest を入れて UT を書くところから始めてみた。というか UT をあーだこーだするのに何時間か使って、実際のコードのリファクタリングは5分くらいで終わってしまったw
スライドどれもおもしろいー。Q&Aのとこは色んなことを知っていてすごいなー。
— Mitsuyuki Shiiba (@bufferings) August 4, 2022
「テストコードにはテストの意図を込めよう」の発表報告&補足説明&質問回答 #vstat - ブロッコリーのブログ https://t.co/sNAK2hjfrc
その1:最初に書いた UT
Xが左から右に、Yが上から下に増えていく座標系なのねって思いながら、とりあえずパラメタライズドテストでやってみるかーって書いたもの:
import * as targetFunctions from './detectMoveDirection'; describe.each(Object.values(targetFunctions))('%p test', (targetFunction) => { test.each` prevX | prevY | nextX | nextY | expected ${20} | ${20} | ${30} | ${30} | ${{ horizontal: 'right', vertical: 'bottom' }} ${20} | ${20} | ${10} | ${30} | ${{ horizontal: 'left', vertical: 'bottom' }} ${20} | ${20} | ${20} | ${30} | ${{ horizontal: 'neutral', vertical: 'bottom' }} ${20} | ${20} | ${30} | ${10} | ${{ horizontal: 'right', vertical: 'top' }} ${20} | ${20} | ${10} | ${10} | ${{ horizontal: 'left', vertical: 'top' }} ${20} | ${20} | ${20} | ${10} | ${{ horizontal: 'neutral', vertical: 'top' }} ${20} | ${20} | ${30} | ${20} | ${{ horizontal: 'right', vertical: 'neutral' }} ${20} | ${20} | ${10} | ${20} | ${{ horizontal: 'left', vertical: 'neutral' }} ${20} | ${20} | ${20} | ${20} | ${{ horizontal: 'neutral', vertical: 'neutral' }} `('($prevX, $prevY) -> ($nextX, $nextY)', ({ prevX, prevY, nextX, nextY, expected }) => { expect(targetFunction(prevX, prevY, nextX, nextY)).toStrictEqual(expected); }); });
targetFunctions
のところは気にしなくて大丈夫。detectMoveDirection.js
ファイルに新しい関数を書いたらそれも全部同じテストを通るようにしただけで、今日の記事のためだけのものなので、これ以降の例では省略する
例えば WebStorm で実行するとこんな感じになる
これくらいの関数だったら、別にこのくらいのテストでもいいかなって思う。んだけど、色々遊んでみた
そうそう、僕はあんまり期待値をテストメソッド名に書かない人です。条件だけ書くことが多い(現在のところ)
その2:ちょっと変えてみる
要らなさそうな情報があるから削ってみようかな
prevX
とprevY
は変わらないので、パラメータから除外して値を直接埋め込んでみるexpected
に Object じゃなくてそれぞれの値を書いてみる
するとこうなった
test.each` nextX | nextY | expectedHorizontal | expectedVertical ${30} | ${30} | ${'right'} | ${'bottom'} ${10} | ${30} | ${'left'} | ${'bottom'} ${20} | ${30} | ${'neutral'} | ${'bottom'} ${30} | ${10} | ${'right'} | ${'top'} ${10} | ${10} | ${'left'} | ${'top'} ${20} | ${10} | ${'neutral'} | ${'top'} ${30} | ${20} | ${'right'} | ${'neutral'} ${10} | ${20} | ${'left'} | ${'neutral'} ${20} | ${20} | ${'neutral'} | ${'neutral'} `('(20, 20) -> ($nextX, $nextY)', ({ nextX, nextY, expectedHorizontal, expectedVertical }) => { expect(targetFunction(20, 20, nextX, nextY)).toStrictEqual({ horizontal: expectedHorizontal, vertical: expectedVertical, }); });
んー。まぁ悪くはない・・・prevX
と prevY
をなくしたからスッキリはしたものの、どんな値を使ってるの?って一瞬探してしまうから、そんなに変わんないか。どっちかっていうと最初の方が好きかも
その4:意図を書いてみる
今度は意図を書いてみよう:
describe('Xが増加', () => { test('Yが増加', () => { expect(targetFunction(20, 20, 30, 30)).toStrictEqual({ horizontal: 'right', vertical: 'bottom', }); }); test('Yが減少', () => { expect(targetFunction(20, 20, 30, 10)).toStrictEqual({ horizontal: 'right', vertical: 'top', }); }); test('Yが変わらない', () => { expect(targetFunction(20, 20, 30, 20)).toStrictEqual({ horizontal: 'right', vertical: 'neutral', }); }); }); describe('Xが減少', () => { test('Yが増加', () => { expect(targetFunction(20, 20, 10, 30)).toStrictEqual({ horizontal: 'left', vertical: 'bottom', }); }); test('Yが減少', () => { expect(targetFunction(20, 20, 10, 10)).toStrictEqual({ horizontal: 'left', vertical: 'top', }); }); test('Yが変わらない', () => { expect(targetFunction(20, 20, 10, 20)).toStrictEqual({ horizontal: 'left', vertical: 'neutral', }); }); }); describe('Xが変わらない', () => { test('Yが増加', () => { expect(targetFunction(20, 20, 20, 30)).toStrictEqual({ horizontal: 'neutral', vertical: 'bottom', }); }); test('Yが減少', () => { expect(targetFunction(20, 20, 20, 10)).toStrictEqual({ horizontal: 'neutral', vertical: 'top', }); }); test('Yが変わらない', () => { expect(targetFunction(20, 20, 20, 20)).toStrictEqual({ horizontal: 'neutral', vertical: 'neutral', }); }); });
んー。長くなったなぁwでも意図は分かるのはとても良い。実行結果はこうなる
実は、↑の前に、こんなパラメーター化をしてみた↓んだけど、あんまり好きじゃなかったので却下した。これが「その3」
describe.each` caseName | nextX | expectedHorizontal ${'Xが増加'} | ${30} | ${'right'} ${'Xが減少'} | ${10} | ${'left'} ${'Xが変わらない'} | ${20} | ${'neutral'} `('$caseName', ({ nextX, expectedHorizontal }) => { test.each` caseNameY | nextY | expectedVertical ${'Yが増加'} | ${30} | ${'bottom'} ${'Yが減少'} | ${10} | ${'top'} ${'Yが変わらない'} | ${20} | ${'neutral'} `('$caseNameY', ({ nextY, expectedVertical }) => { expect(targetFunction(20, 20, nextX, nextY)).toStrictEqual({ horizontal: expectedHorizontal, vertical: expectedVertical, }); }); });
その5:それぞれのケースでいくつかの値をチェック
最初のテストだとあんまり気にならなかったけど、意図を書くと「あぁ、それぞれの意図の中にいくつかのケースを書きたいなぁ」って気持ちになった。ゼロ周りとか、マイナスとか。数値の最大最小値周りは…今日はいいや。みたいなことを考えてこう書いてみた:
describe.each` prevX | nextX ${10} | ${15} ${0} | ${1} ${-1} | ${0} ${-15} | ${-10} `('Xが増加($prevX -> $nextX)', ({ prevX, nextX }) => { test.each` prevY | nextY ${10} | ${15} ${0} | ${1} ${-1} | ${0} ${-15} | ${-10} `('Yが増加($prevY -> $nextY)', ({ prevY, nextY }) => { expect(targetFunction(prevX, prevY, nextX, nextY)).toStrictEqual({ horizontal: 'right', vertical: 'bottom', }); }); test.each` prevY | nextY ${15} | ${10} ${1} | ${0} ${0} | ${-1} ${-10} | ${-15} `('Yが減少($prevY -> $nextY)', ({ prevY, nextY }) => { ...(省略)
んー。色んなパターンがテストできていいけど、読みやすくはないかぁ
その6:もうちょっと読みやすくしてみた
X と Y のパターンを掛け合わせなくてもいいかなぁって思ったので、 組み合わせごとに describe を書いてみた。そして、これでいいかーって気持ちになった
describe('Xが増加・Yが増加', () => { test.each` prevX | prevY | nextX | nextY | expectedHorizontal | expectedVertical ${20} | ${20} | ${30} | ${30} | ${'right'} | ${'bottom'} ${0} | ${0} | ${1} | ${1} | ${'right'} | ${'bottom'} ${-1} | ${-1} | ${0} | ${0} | ${'right'} | ${'bottom'} ${-20} | ${-20} | ${-10} | ${-10} | ${'right'} | ${'bottom'} `( '($prevX, $prevY) -> ($nextX, $nextY)', ({ prevX, prevY, nextX, nextY, expectedHorizontal, expectedVertical }) => { expect(targetFunction(prevX, prevY, nextX, nextY)).toStrictEqual({ horizontal: expectedHorizontal, vertical: expectedVertical, }); } ); }); describe('Xが増加・Yが減少', () => { test.each` prevX | prevY | nextX | nextY | expectedHorizontal | expectedVertical ${20} | ${30} | ${30} | ${20} | ${'right'} | ${'top'} ${0} | ${1} | ${1} | ${0} | ${'right'} | ${'top'} ${-1} | ${0} | ${0} | ${-1} | ${'right'} | ${'top'} ${-20} | ${-10} | ${-10} | ${-20} | ${'right'} | ${'top'} `( '($prevX, $prevY) -> ($nextX, $nextY)', ({ prevX, prevY, nextX, nextY, expectedHorizontal, expectedVertical }) => { expect(targetFunction(prevX, prevY, nextX, nextY)).toStrictEqual({ horizontal: expectedHorizontal, vertical: expectedVertical, }); } ); }); describe('Xが増加・Yが変わらない', () => { test.each` ...(省略)
実行結果
これでいっかー!
テストを色々書いて楽しんだので
あとは jest で watch
オプションをつけてプロダクションコードをリファクタリングするだけだった。watch
オプションをつけておくと、ファイルを更新するたびに関係するテストが再実行されるので便利。いっかいコードを書き間違えてテストが落ちたので、おぉ!って思いながら修正したのだ
おもしろかったー!
今回みたいなシンプルな関数に対するテストでは、「その1」で十分だと思う。もうちょっと条件とかが複雑で細かく挙動をチェックしておきたい場合は、「その6」にしたい気持ち
こういうときに、QAエンジニアがいてくれると「どんなケースを残しておくといいのかな?」って相談できて良いんだろうなぁと思うのだった
コードはここに置いといた: https://github.com/bufferings/detect-move-direction-js
おわりー