盛大な手戻りを防ぐため、先にテストケースだけレビューしてもらう開発手法
TL;DR
- デキる人は完成形を見せてから詳細を進めていく
- 30%ぐらい完成したタイミングで完成イメージを一度見せて、齟齬がないことを確認すると良い
- コードも完成イメージを一度レビューしてもらってから細部を実装した方が良いのではないか
- テストケースだけ先に見てもらうと良いんじゃない?
仕事がデキる人はなぜ〇〇なのか
「仕事がデキる人はなぜ〇〇なのか」みたいなビジネス書によく書いてあるテクニックがあります。それは
頼まれた仕事は30%ぐらい完成したタイミングで一度依頼主に見せて、認識齟齬がないことを確認すること
よく出てくるのがプレゼンの例です:
仕事がデキないパターン
上司「今度の企画案を部長に説明するためのプレゼンを作っておいてくれ。プレゼンは1週間後ね」
部下「わかりました!」
〜1週間後〜
部下「できました!」
上司「なにこれ!全然思ってたのと違う!」
部下「やっちまったなぁ!」
仕事がデキるパターン
上司「今度の企画案を部長に説明するためのプレゼンを作っておいてくれ。プレゼンは1週間後ね」
部下「わかりました!」
〜1時間後〜
部下「こんなイメージで作ろうと思うのですが、如何でしょう?」
上司「この辺はちょっとこういう感じに変えておいてくれ」
部下「ええよ」
〜3日後〜
部下「あとは図を埋めたら完成なんですが、図を作るのは時間がかかるので事前に見てもらえないでしょうか?」
上司「ふむふむ...この図は要らないかな。逆にこういう図を作ってほしい」
部下「おっけー」
〜1週間後〜
部下「できました!」
上司「イメージ通り!何この新人くん超最高なんですけど」
仕事がデキないパターンをやっちまってないか?
「途中で確認すること」はあらゆる仕事の基本ですが、コードを書く時この基本を見落としてしまっているケースを意外と見かける気がします
事前に一度の相談もなく3日ぐらいかけて 36 files changed の鬼PRに「レビューお願いします!」の一言を添えて提出したら
- 「全然既存のアーキテクチャに沿ってない、書き直して」と突き返される
- 「仕様ちゃんと見た?」と突き返される
- 「既存の共通処理あるから、そっち使って」と突き返される
もしこういうパターンに陥っていたら、一度仕事の進め方を見直すチャンスかもしれません!
コードを途中で確認してもらうには
とはいえ
const newHoges = hoges.map(( // 途中まで書いた!
こんな中途半端な状態で「途中までできました!確認お願いします!」と提出してもレビュワーにシバかれて終わるだけなので、何かしらレビュワーにとって理解しやすい、キリの良いところで提出したいですよね
そこで提案したいのが 「テストケースだけ」 書いたPRを見てもらう方法です
こんなイメージです
(hoge.test.js)
it('引数に1を渡したら2が返る', () => {})
it('引数に2を渡したら例外が発生する', () => {})
it('引数に42を渡したら全宇宙の真理とコンソールに出力される', () => {})
この状態で一度テストケースだけでもレビューして貰えば、少なくとも自分がこれから実装しようとしている機能の仕様面については認識が揃っているはずです。
もちろんテストケースで表現しきれない要件や非機能要件もあるので完璧な認識確認にはなりませんが、テストケースという完成図を一度見せておいて途中確認する人と、全く確認せず実装をガンガン進めて盛大にやり直しをくらう人がいた時、周囲から見ると恐らく前者の方が安心して仕事を任せられるのではないでしょうか?
テストの中身は書かなくて良いの?
一度テストケースを見てもらってからテストの中身を書いても良いと思いますが、テストが落ちるコードをpushすることに対する考え方はチームによって異なるので、一度チームと相談した方が良いと思います。
「テストが落ちるたびSlackに通知が飛ぶからやめてくれ!」と嫌がられるかもしれませんし、逆に「要件を満たしていないテストが通過し続けているとテストの実装を忘れるかもしれないから、落ちても良いのでテストの中身を書いておいてほしい!」と言われるかもしれません。個人的にはテストが落ちている限りマージできないようになっていればガンガンpushして良いのでは?と思いますが
ついでに実装イメージも添えてあげる
「こんなイメージで実装しようと考えていますよ」と共有できるような仮実装を添えてあげるとレビュワーもほっこり幸せになるかもしれません。以下のようにクラスのインターフェースやメソッドの意図を書くだけで、詳細実装はこの段階では書きません。どこにどんなコードをこれから書こうとしているのか伝えるのが目的です。
class User {
getAge() { return 20 } // todo: 今は定数だけど実際は生年月日から計算して年齢を返す
addAddress(address: string) {} // todo: 有効な住所か判定します。住所一覧に新しい住所を追加します。5件以上は登録できないようにここで制御します
}
コードベースが大きくなってくると「そういうコードはこの層に書いてね」とか「そのコードからこのコードを触っちゃダメだよ」とか、設計に関係する大きな指摘が増えてきます。何十時間もかけてからそういう指摘を受けると盛大な手戻りが発生しますが、設計などの観点は仮実装の段階でも十分に見れるので、積極的に仮実装の状態でチェックしてもらいましょう。
仮実装に合わせて少しだけテストケースも実装を進めてコードの使い方をイメージできるようにしてあげるとレビュワーが更にほっこりするかもしれません
it('addAddressの引数に渡した文字列(住所)がlistAddressで取得できる', () => {
const user = new User()
user.addAddress('hoge')
// todo: user.listAddressしたら ['hoge'] が返ってくるイメージ
})
こんな感じで、まずはテストケースだけ見てもらい、実装イメージを見てもらい、そこから詳細実装を進めていく・・・みたいな感じで、大枠の完成図から徐々に詳細を埋めていく様はまさに冒頭紹介した「仕事がデキるパターン」ですね!
もし難しい機能実装を任されるようになって進め方に困っている方がいたら、こんな仕事の進め方を意識してみると良いかもしれません。
Discussion