OOP前回のエントリではレガシープログラマの判断項目について、書きました。その日、仕事でレガシープログラマさんの一人が書いたプログラムを一緒にリファクタリングしました。レガシープログラマさんと言っても、おいらより年下の女性エンジニアです。今回のエントリではそのやりとりについて書いてみたいと思います。元のプログラムはどんなプログラム?そのプログラムは以下の判断項目に該当していました。複数のデータをまとめて扱う際は毎回配列を使う。配列の上限数はありえなさそうな数を指定する(1000とか)。基本データ型(stringやint)と配列だけでデータ構造を表現しようとする。クラスのフィールド変数をグローバル変数のように利用する。言語はC#2.0で、CSVを読み込んでメールを送信するプログラムです。同じFromとToの組み合わせに対しては一通のメール内の複数のコンテンツを含めて送信します。イメージ的にはこんな感じです。こんばんはこのCSVの場合、3通のメールが送信されます。1件目と2件目はFromとToが同じなので、一つのメールに「おはよう」と「こんにちは」を含めて送信します。次にプログラムの改善前と改善後を示します。ちなみにこの案件は新規開発ではなく、既存プログラムの機能追加です。実際はもっとややこしい感じで、コメントもほとんどありません。}}mailInfoTableというフィールド変数が複数のデータをまとめて扱う際は毎回配列を使う。配列の上限数はありえなさそうな数を指定する(1000とか)。基本データ型(stringやint)と配列だけでデータ構造を表現しようとする。クラスのフィールド変数をグローバル変数のように利用する。いうアンチパターンをすべて兼ね備えてしまっています。そのため、ReadCsvLinesメソッドやSendMailFromMailInfoTableメソッドの実装が分かりにくいものになってしまっています。columns[2];」というように複数のコンテンツを文字列で区切って格納しているところも強引というか、涙ぐましい工夫を凝らしたロジックになっています。T)また、ReadCsvLinesメソッド内で同一のMailFromとMailToを探すロジックも、毎回配列をループで回しながら検索するので非効率です。これを大体一時間ぐらいかけて下のようにリファクタリングしてみました。}}まあこの状態でも色々とツッコミどころはありますが、改善前よりかは可読性や堅牢性が増したはずです。ポイントは以下の通りです。同一のFromとToを検索するのに、ディクショナリのキーを利用した点レガシープログラマさんの反応は?リファクタリングはペアプログラミング方式というか、おいらが横について説明し、プログラムの入力はレガシープログラマさんにやってもらいました。一通りコーディングと動作確認を行ったあとに、最後にまとめてロジックの説明しました。自分でクラスを作ってデータを格納するようなプログラムはおそらくそれまで作ったことがないと思うので、理解してもらえるかどうか不安でしたが、大丈夫だったようです。紙の上に図を書いたりして丁寧に説明したのが良かったのかもしれません。最後にぽつりと「感動しました」と言われました。おいらも昔、すごい先輩の技術を目の当たりにして感動した事が何度かあります。した体験が技術力を向上させたいと思う大きな原動力になりました。おいらもそういう先輩のように誰かの手本になりたいと思いながら勉強してたりするので、非常に嬉しかったです。2種類のレガシープログラマレガシープログラマの中にも昔のやり方に固執しようとするタイプの人と、単にモダンなプログラミングスタイルを知らないだけの人がいると思います。前者の人とはぶつかりやすいかもしれませんが、後者の人はまだまだ伸びしろが大きいと思うので、こういった機会をもっと増やしていきたいですね。先月、CodeIQにビンゴカード作成問題を出題しました。はてなブログPro株式会社ソニックガーデンで働くRubyプログラマ。2017年11月に「プロを目指す人のためのRuby入門」を出版。blog)はてなブログをはじめよう!JunichiItoさんは、はてなブログを使っています。ブログを報告するスターをつけました引用をストックしましたストック一覧を見る閉じる引用するにはまずログインしてくださいログイン閉じる引用をストックできませんでした。再度お試しください閉じる限定公開記事のため引用できません。