読者です 読者をやめる 読者になる 読者になる

Blank?=False

「呉下の阿蒙にあらず」をモットーにしたITエンジニアの日々

リファクタリングの帽子をかぶるということ

f:id:stonebeach-dakar:20170206191659p:plain
みなさんこんばんわ。
VBAフォルダの再帰的な作成をしようとしたところ、標準機能ではできなかったので、 APIは使いたくないのでネットで探したコードを使いました。

そのコードが、そのままだとちょっと微妙かなと思ったのでリファクタリングをしました。

今回は、そんなネットで拾ったコードとリファクタリングの話について書いていきます。

元のソースコード

f:id:stonebeach-dakar:20170206191746p:plain
こちらのページのコードです。
VBAメモ Windows APIを使わずに階層の深いフォルダーを作成してみる - tetsuyanbo

とりあえずこうかな、位の軽い感じで書かれたコードだと思います。
(作者様、誠に勝手ながらGistに上げさせて頂きました。不都合あれば削除致します。)

再帰的にフォルダを作成 http://www.tetsuyanbo.net/tetsuyanblog …

コードをそのままコピペしてはいけない

こちらのコードをそのまま使うと、他のコードとの親和性が微妙ですしちょっと冗長にも感じます。
そして、そのまま使うというのはコピペ開発と言われ、 多くのプログラマから忌み嫌われています。

コピペ駆動開発をするのは職業プログラマならできるだけ避けておいたほうがいいです。
ソースコードレビューのときにこのコードはこういう動きができます、とすぐに説明できないものは、 もしそこでバグが発生した時にどこが原因かの割り出しに時間がかかってしまいます。

こんな記事もあります。
d.hatena.ne.jp

ちなみに、まるまるコピペしたな、っていうのは他のコードと変数名の付け方などの違いで意外とわかってしまうので、 バレてないと思っても実はバレていたりします。


というわけで、このコードをこんな風にリファクタリングして使うことにしました。

再帰的にフォルダを作成

どうリファクタリングしたか

f:id:stonebeach-dakar:20170206191721p:plain

まず、目についたのが変数名

様々な変数の先頭ににstr,obj等のそのクラスや型などがついています。
これは、プリフィックスと言いまして、プリフィックスを変数名の先頭につける書き方を世間一般ではハンガリアン記法と呼びます。
が、実は本来のハンガリアン記法ではないのです。 詳しくはWikipediaに書かれています。

マイクロソフトのアプリケーション開発グループで開発されたこの記法は Excel や Word などの開発で成功を収めたため、 Windows 開発グループでも採用された。その際、シモニーの論文中の “type” がデータ型のことであると誤って解釈され、 変数名にデータ型を表す接頭語や接尾語をつける記法だと誤解された。
シモニーの意図していた記法をアプリケーションハンガリアン、誤解に基づく記法をシステムハンガリアンと呼ぶ。
ハンガリアン記法 - Wikipedia

この「システムハンガリアン」は変数名が冗長になりやすく、逆に読みにくくなることがしばしばあります。
例えば、ファイルパスを格納する変数名をstrFilePath、もしくはFilePath,どちらが読みやすいでしょうか?

まず、ファイルパスというのは間違いなく文字列です。
としたら、わざわざ文字列と表現するstr接頭語はいりません。なくたって文字列型と分かります。
逆に、文字列型でなければそもそもの付け方がおかしいと思います。

そして、この接頭語があるから大丈夫、を免罪符に分かりにくい変数名になっているものをしばしば見かけることがあります。

例えば、strFileです。
これは、ファイル名を格納する変数です。
ファイル+文字列ならファイル名でしょ、という考えで付けられたものですが、ファイルと文字列で想像出来るものは他にもファイルパス、ファイルの作成者等色々なことが文字列で表現出来るため、「わかりにくい」になってしまうのです。


この考えに従い、各変数を見直しました。

使わない変数がある

FileSystemObject.CreateFolderの返り値を格納するstrValueという変数があります。
この変数、使っていないんですよね。入れて終わり。

こういう使わない変数は読みにくくなりやすく、またメモリを余分に消費してしまいます。
こういう使わない変数は消してしまいます。

IF文をワンライナーで書く

自分の場合、IF文はできるだけワンライナーで済むように書きます。
これは、ネスト(深さ)を浅くするため、そして読みやすくするためです。

ワンライナーで書くことで、IF ◯◯ then △△もし◯◯なら△△する と直感的に読みやすくなります。
これをブロックで構成されると、もし◯◯条件であればこのブロック内を実行、と考えて、次にブロック内は△△をする、と考える必要になってしまいます。

このため、私の場合大抵はIF文はワンライナーを心がけ、もし無理そうならブロック内の処理を別メソッドに抽出するなどして ワンライナーにしています。

遅延バインディングを事前バインディングに直す

これはVBA特有?VBも?かはわかりませんが、外部ライブラリを使う時に遅延バインディング,事前バインディングの2つのやり方があります。
それぞれメリット、デメリットがありますが、それについては以下を参照してください。

excel-ubara.com


遅延バインディングにメリットがないように書かれていますが、実際のところは複数環境での使用が想定されるときは 遅延バインディングを使うことで互換性対策ができることがあります。

ですが、余りないケースなので私の場合基本的に事前バインディングを使います。

リファクタリングの参考書籍

f:id:stonebeach-dakar:20170206191947p:plain
実に様々な書籍でリファクタリングが紹介されていますが、この1冊読めばOK!という本は 間違いなくマーチン・ファウラー氏のリファクタリングしかないでしょう。

新装版 リファクタリング―既存のコードを安全に改善する― (OBJECT TECHNOLOGY SERIES)

新装版 リファクタリング―既存のコードを安全に改善する― (OBJECT TECHNOLOGY SERIES)


この本は、最初はリファクタリングとはなんぞや、なぜリファクタリングするのか、等から始まり、 中盤からはリファクタリング手法を紹介するカタログになります。
私は前の会社にいた時にこの本が会社にあったので読んだのですが、非常にためになる内容で、読んだ日からソースコードが大分綺麗になった記憶があります

今は手元にないのですが、いつか購入する予定です。
カタログを半分くらい忘れてしまっているのもあるので、改めて読み直しておきたいと考えています。

ちなみに、本記事のタイトルであるリファクタリングの帽子をかぶる、と言うのは本書に「何かアレと思ったらリファクタリングの帽子をかぶって見てみる」という所から来ています。


このリファクタリングですが、Rubyエディションもあります。
この本は読んだことがないのですが、レビューを見ると原書そのまま、Ruby言語に直したような雰囲気のようです。
(原書はJavaです)

リファクタリング:Rubyエディション

リファクタリング:Rubyエディション