読み始める前に
あのときの気持ちを忘れないために記事として残しておきます
この記事は自分が1年以内に参画したVB.NETプロジェクト内で遭遇した、地獄のようなバッドプラクティス集です。絶対真似しないでください。
1.プロジェクトの設定がOption Strict Offになっている
問題点
- 暗黙的な型変換が行われるようになり、実行前に防ぐことのできるバグが実行するまでわからなくなる。
- 型が違う場合は実行時に勝手にキャストされてしまい、バグ発生量、デバッグ量がとっても増えるので絶対にやめよう**(実際にDecimal型の値がInt型に勝手にキャストされて大変なことになった)**
対処
- Option Strict Onにしよう
- 自動でのキャストに頼らず、必要なときはDirectCast関数やCInt関数等を使って明示的にキャストしましょう
2.社内フレームワークの関数の戻り値がObject型(Valiant型)
問題点
- 何が戻ってきているのかわからないので、VisualStudioの補完が一切きかない。
- 文書や、デバッグモードで実際の型が何なのか確認する必要が出てくる。
- 大量にキャストが必要になる、そしてOption StrictがOffにされる
- 作業量が増えまくるので絶対にやめよう
対処
返したい型のままで返して
(そのフレームワークは元がVB6用で、VB.NETにそのままリプレースしたので、その名残が残っているらしい)
3.メソッドの実行結果がすべてフィールド変数に格納される
問題点
分かりづらい、使いにくい、フィールド変数の数が多くなればなるほど、どの変数がどのメソッドの戻り値なのかわからなる
対処
- 基本、返したい値があるなら戻り値で返す。
- 複数の値を返す必要があるならデータクラスを作って、それに格納し戻り値で返す。
- もし、処理の成功、失敗があり、成功の場合には値を返すようなメソッドであれば、処理の失敗時に例外をスローするような設計も考慮に入れる
4.クラスのフィールド変数の初期化をクラスの利用者側でやらないといけない
実装例
Dim database = New Database()
ReDim database.SelectedData(99)
'SelectHogeメソッドのデータ格納用フィールド変数(配列)の初期化
database.SelectHoge()
Dim hoge = database.SelectedData
'hogeをいろいろ使う
問題点
カプセル化なんてなかったんや。。。
非常に使いにくい、初期化する変数が多ければ多いほど、使うための手間が多くなる
対処法
- オブジェクト指向を勉強しなさい
- 今回の例はメソッド側でSelectedDataを初期化すれば解決する問題だよね
- それ以前に取得してきたデータは戻り値で返しましょうね
5.Rnd関数を使う
例
Function NextInt(min As Integer, max As Integer) As Integer
Return (max - min) * Rnd() + min
End Function
問題点
- Randomize関数を実行し忘れると、アプリケーション再起動時に同じ値が出てくる
- 車輪の再発明でバグの原因になるかもしれない
対処
- .NETframeworkにはRandomクラスがあるのでそっちを使う
- RandomクラスにはNextメソッドがあり、範囲指定ができるので上記の関数を実装する必要がない
- まず先に.NETframeworkやライブラリを確認して、解決する方法がないか探して見ることが重要
#6.ほしい値にたどり着くまでがとても長い
例
Dim hoshiiValue = dicHoge(dicFuga(FugaKeys.FugaFuga))(0).HogeArray(HogeLabel.HogeHoge)(0).HogeFuga
'確か顧客データのリストだったはず
問題点
- こんなの作った人にしかわからない、覚えきれない
- 実際はこれより長くて、しかも上司のレビューを通過していた
- こんな構造を作ってしまった人はデータクラスという概念を一切しらなかったっぽい
対処
- データクラスを作って1件分のデータをまとめ、それをリストなどに格納していって、クラスや関数に渡す
- 関数やクラス、メソッドには必要な範囲のデータを渡すようにする(過剰に渡さない)
7.コーディングルールに関数やメソッドは必ずTry Catchで囲むがある
例
Sub FuncHoge(ByRef value As User)
Try
'何かいろいろ処理
Catch e As Exception
'握りつぶす
End Try
End Sub
問題点
- バグが発生した場合の原因究明がとても困難
- エラー時の処理が各所に散らばっていて、修正がとても困難
- こんなコーディングルールを設定するプロジェクトマネージャは、例外処理をおじゃま虫としか思っていないんだろうな
対処
- プログラマーの人は何でもかんでもcatchしない、大抵はthrowしたままで大丈夫なはず(もし、catchする必要があるなら、しっかり確認を取る)
- Exceptionでキャッチは基本しない(SocketExceptionが発生するなら、SocketExceptionでキャッチする等)これも要確認
- プロマネの人はIO・ネットワーク等の例外をどこでcatchして処理するか、しっかり考える
最後に
1から5までの問題を発見したプロジェクトは、開発環境も良くなくVisualStudio2008で、更にコーディングルールもVB6時代を踏襲したままで、.NETFrameworkも2.0までと悪い状況の詰合せみたいな感じでした。出そうと思えばもっと最悪なことがいっぱいありますが今回はこれぐらいに。