日本におけるテスト駆動開発(TDD)のスペシャリスト。)grooves、Aiming、Quipperなどでリーダー的な活動をしていたプログラマー。RubyとHaskellが得意。フロントエンド開発の実績も豊富。好きな寿司:コハダそれでは審査結果の発表です。さんの2つの投稿です。特色が遺憾なく発揮されており、リファクタリング前のコードと比べて著しく可読性が改善されている点が印象に残りました。ひとつ気になるのは、リファクタリング前の実装には存在していた副作用がなくなっている点です。リファクタリング前のコードでソートされるのは戻り値だけでなく、引数に渡した配列も破壊的にソートされます。リファクタリングで大事なのは「外部から見た振る舞いを変えずに、内部を改善する」ことなのですが、この副作用は、はたして外部から見た振る舞いでしょうか?今回は、引数に及ぼす副作用は、意図していないものであろうと判断しました。理由は、リファクタリング前のテストに引数の副作用に関するテストが無いためです。テストに書いていないということは、副作用は意図したものでない、という判断です。機能互換性に関する判断を行うことができます。(このあたりの設計判断も回答に書かれているとなお良かったと思います)などなど、様々な意味で、この投稿は印象に残りました。副作用を起こしながら結果の配列を作るという、不変性とは何だったのか的なコードでテンションが上がりました。トリッキーだと感じる人もいるかもしれませんが、僕は簡潔で読みやすいコードだと思います。いうことであまり指摘するところが無いんですが、強いて言えばcという変数名はa、bの次にあるものを連想させてミスリーディングかもしれないので、sortedとかにするといいかなあと。コメントバグを出さないおそらく唯一の方法は、プログラムを自分で書かないことです。チームでのアプリケーション開発では、フレームワークやライブラリの機能を理解してうまく使いこなして自前で実装するコードを減らすスキルはとても重要です。この投稿のAfterのコードは誰がどう読んでもなんの面白みもヒネリもない平凡すぎるコードなんですが、チームメンバーから見るとそんなコードが一番尊いのではないでしょうか。しておかないとヌルポの可能性がありそうですね。コメント何か特別すごいテクニックが使われているわけでもないんですが、生々しくて実践的な感じがよいですね。Searchingというモジュール名は、「検索」という意味だろうとは思いますが、現在進行形だと「探してる」みたいなニュアンスになってしまって違和感があります。コメントこの投稿が良かったのは、レガシーコードのリファクタリングの過程を公開している点です。ここで言う「レガシーコード」とは、テストコードが書かれていないコードのことです。https://github.com/kenchan/forkwell-jobs-sushi-contest-2015リファクタリングは、小さく安全なステップで対象のコードを段階的に綺麗にしていくプロセスです。安全なステップの「安全」を支えるのはテストです。レガシーコードはテストが書かれてこなかった故に、テストが書きにくい、あるいはそのままではテストが書けない構造になっていることが多々あります。リファクタリングにはテストが必要だが、テストを書くためにはリファクタリング(構造変更)が必要、という一種のデッドロックに陥ります。このデッドロックに直面したとき、私たちはテストを書けるようになるまでテスト無しで構造変更するか構造変更を伴わず強引に大外からテストするという二つの道のどちらかを選択します。さんの選んだ道は前者で、それは対象コードが十分に小さいから、という判断も働いたのでしょう。公開されている履歴を辿っていくとテストが書けない構造をテスト可能にするために最小限の変更を行う小さなテスト基盤を整えてテストを必要なだけ記述そのテストを通しながら内部を改善する安全圏に達したら、必要に応じて段階的に対象コードの(外部も含め)構造変更を行うというレガシーコード改善の王道とも言える進め方をしている点が印象に残りました。コメントベタ書きだった処理が、適切な範囲の責務を持つクラスに切り分けられていく。小さな例ですがオブジェクト指向リファクタリングのあるべき姿を示していると感じました!それでいて冗長になっていないのが個人的に非常に好みです。}のHashをlookupして終わりにしちゃうところですが、このくらい簡潔に書けるならクラスにしてもいいなあと。指摘するとしたら、TimeOfLifeはLifeStageのほうが通りがよさそう。コメント※同氏はこちらのコードを次点としておりましたが、その選出時のコメントを掲載しています。1つぐらいはRubyの新機能を活用したものを採りたかったのでこれを選びました。この内容ならクラスを作るまでもないだろうという意見もありそうですが、そのあたりは敢えてそういう設計を選んだようですし、そういった方向性も含めて、全体的な仕事の丁寧さに好感が持てます。コミットログをしたためてくれたのは素晴らしいんですが、日本語コミットはちょっと読む気しないのでプラマイゼロ。lifeの複数形はlivesなような気がするのでそのあたりが減点対象で、そもそもそういう紛れがありそうな名前を避けるのもスキルのうちかもしれません。今回、Ruby賞、審査員賞それぞれで審査員の意見が一致しましたので、特別にもう一つ賞をご用意しました。ブロックを使って、繰り返し使うコードをメソッドに切り出すです。※商品は審査員特別賞と同じものを贈呈いたします。コメント僕も同じようなコードを半年前に書いたので懐かしい思いで見させて頂きました。繰り返し処理の抽象化により、コードがあるべき方向に進んでいると感じました。実態は繰り返し処理をして値を返すメソッドなので、paginate(ページを付ける)だとミスマッチかも。定義して、それによるストリーム処理にするとよさそうです。いう風に書けると、馴染みがよくなりそうです。オーバーデザインだ!と言われる気もしますが。プログラムはチームで開発している事が多いので、このように議論のスタートになるリファクタリングはとても価値が高いと個人的には思っています。コードレビューを通して完成形に近づけばよいので。さんには、ネスカフェコーヒーメーカーをプレゼントします。参加賞コードをきれいにするアレを手に入れたのはこちらの3名ですhanachin_yancya5t111111すべての受賞者には、Forkwellより個別にご連絡させていただきます。開発者が主役となって活躍できる会社を増やし、そんな会社と開発者が出会うきっかけを創造したいと考えています。そのために、開発環境をしっかりと整備し、公開できる会社の求人だけを厳選して掲載しています。