2011-02-18

レガシープログラマかどうかを判断する10項目

OOP C#

※2011.3.30追記
11個目の判断項目を追加しました。
また、「昔はね...」の補足説明を各項目に追加しました。

レガシープログラマ = モダンな言語のおいしい機能をうまく使いこなせていないプログラマ

おいらは時々社内システムのコードレビューなんかをやっているのですが、「なんかちょっと前時代的だな〜」とか「ちょっと修正したらC言語でもコンパイルできそうだな〜」って思うことがよくあります。

おいらがレビューする言語は主にC#です。C#やJavaのような比較的モダンな言語のおいしい機能をうまく使いこなせていないプログラマを、ここでは「レガシープログラマ」と呼ぶことにします*1。

そこで、おいらがこれまでに見てきたコードの中から「これはレガシープログラマっぽい」と思った典型的な症例を~~10個~~11個挙げてみます。

レガシープログラマの判断項目

使われるローカル変数をすべてメソッドの最初に宣言する。
ローカル変数の宣言時に空文字("")や新しいオブジェクト(new Xxx())で初期化する。その後にすぐ別の値をセットする。
メソッドの戻り値がすべて成功・失敗を表す 0 か -1 になっている。
複数のデータをまとめて扱う際は毎回配列を使う。配列の上限数はありえなさそうな数を指定する(1000とか)。
基本データ型(stringやint)と配列だけでデータ構造を表現しようとする。
変数の命名規則にハンガリアン記法*2を使う。
クラスのフィールド変数をグローバル変数のように利用する。
配列やリストを毎回forループで処理する(例: for (int i = 0; i < array.Length; i++))。
クラスやクラスメンバの可視性を意識していない(privateメソッドがpublicになっている等)。
変更履歴をコード中にコメントとして残す (ADDやDELみたいなコメントがたくさん付いている)。
変数名やメソッド名を何かと略したがる。

判断項目に従って採点してみる

いかがでしょうか？自分の書いたプログラムを見て採点してみてください。

すべて当てはまるようであれば、あなたは立派な「レガシープログラマ」です。
残念ですがJavaやC#を使えているつもりでも、実はほとんど使いこなせていません。
今すぐ中級者〜上級者向けの本を読んで勉強しましょう。

また、該当項目が少なかったといって安心しないでください。
1つでも該当しているようなら、あなたのどこかにレガシープログラマの血が流れています・・・。
下の解説を読んでこの先自分が書くコードを改善していって下さい。

2015.10.26追記：この記事を書いた頃の自分の職場について
この記事を読むと人によっては「xxxみたいな環境だったら今でもこう書く」とか「こういう制約があったらレガシーとは限らない」みたいな異論が出てくると思います。
もちろんそういった異論が出てくるのは理解できるのですが、この記事を書いた時の職場には特に制約はなく、自由にC#のコードが書ける環境でした。

レガシープログラマの処方箋

1. 使われるローカル変数をすべてメソッドの最初に宣言する。

問題点: 変数の寿命が長くなり、可読性が低下する。
改善方法: 変数は必要になったタイミングで宣言する*3。できるだけ変数の寿命が短くなるようにする。
昔はね...: C言語では変数は関数の最初に宣言しなくてはいけなかった。VBも変数の宣言と代入が同時にできなかったので、最初にまとめて宣言されることが多かった。

2. ローカル変数の宣言時に空文字("")や新しいオブジェクト(new Xxx())で初期化する。その後にすぐ別の値をセットする。

問題点: 無駄なオブジェクトが作成され、メモリを無駄遣いする。そのコードを書いたプログラマはオブジェクト参照の概念を理解できていない可能性が高い。
改善方法: 変数は宣言と同時に必要な値をセットする。オブジェクト参照の概念を理解する。
昔はね...: C言語では変数の初期値が決まっていなかったので、必ず初期化する必要があった。

3. メソッドの戻り値がすべて成功・失敗を表す 0 か -1 になっている。

問題点: 戻り値が無視され、予期せぬ不具合を生む。処理結果が引数やフィールド変数に格納され、可読性が低下する。
改善方法: エラー処理のベストプラクティスを導入する*4。
昔はね...: C言語では例外機構がなかったので、処理の成功と失敗を戻り値で返すことが多かった。

4. 複数のデータをまとめて扱う際は毎回配列を使う。配列の上限数はありえなさそうな数を指定する(1000とか)。

問題点: 上限が決まってしまうためにプログラムの柔軟性がなくなる。
改善方法: Listクラスなど、大きさを動的に変更できるコレクションクラスを利用する。
昔はね...: C言語やVBでは配列の大きさを簡単に変更する方法がなかった。

5. 基本データ型(stringやint)と配列だけでデータ構造を表現しようとする。

問題点: 複雑なデータ構造を表現するのが困難になる。無理矢理表現しようとすると複雑怪奇なデータ構造ができあがる。
改善方法: 注文クラスや社員クラスのようなデータをまとめて保持するクラスを作成する。
昔はね...: この問題についてはC言語でも構造体を使えば同じようなことができたはずだが。。。(構造体がネストしたり、構造体に配列を組み込んだりするとポインタの処理がややこしくなったとか？？)

6. 変数の命名規則にハンガリアン記法を使う。

問題点: 変数の型が変更されたら変数名の変更も必要になり、保守性が低下する。基本データ型以外の型が登場すると意味不明なPrefixが生まれる(「Order ordOrder = new Order()」みたいな)。
改善方法: ハンガリアン記法*5を廃止する。基本データ型以外の型やクラスが数多く登場することを理解する。
昔はね...: Microsoftが(誤解に基づいたまま)ハンガリアン記法*6を推奨していた。

7. クラスのフィールド変数をグローバル変数のように利用する。

問題点: 処理の前後にコンテキスト(前提条件)が発生し、可読性が低下する。また予期せぬ不具合が発生しやすくなる。
改善方法: 必要なデータのやりとりは引数と戻り値だけで完結するようにメソッドを設計する。データの数が多い場合は、データをまとめて保持するクラス*7を作成する。
昔はね...: 項目3で説明した通り、戻り値が成功・失敗を表すことが多かったので、戻り値以外の方法(たとえばグローバル変数)でデータをやりとりしていた。

8. 配列やリストを毎回forループで処理する(例: for (int i = 0; i < array.Length; i++))。

問題点: 終了条件を間違えることによって、予期せぬ不具合が発生する可能性がある。
改善方法: イテレータやforeach文など、ループカウンタを使わない処理方法に変更する(「foreach (string val in array)」等)。
昔はね...: C言語ではループ処理を実現する構文と言えばforループとwhileループぐらいしかなかった。

9. クラスやクラスメンバの可視性を意識していない(privateメソッドがpublicになっている等)。

問題点: カプセル化が不完全になり、コードの変更に弱くなる。また、使われていないクラスメンバを見つけにくくなる。
改善方法: 可視性を正しく設定する。最低でもpublicとprivateの区別はつける。
昔はね...: C言語ではpublicやprivateといった可視性をコントロールする機能がなかった。

10. 変更履歴をコード中にコメントとして残す (ADDやDELみたいなコメントがたくさん付いている)。

問題点: コードがコメントだらけになり、可読性が下がる。
改善方法: 変更履歴をコメントとして残さない。履歴はバージョン管理システムとDiffツールで確認する。
昔はね...: バージョン管理システムが今ほど広く普及しておらず、コード中に変更履歴を残すのが最善とされていた。(個人的な推測)

11. 変数名やメソッド名を何かと略したがる。

問題点: 第三者には理解できない変数名やメソッド名であふれかえり、可読性が下がる。
改善方法: 多少長くなっても第三者が理解しやすい名前をつける。どうしても省略したい場合は開発チーム内でルールを決める。
昔はね...: プログラムサイズをできるだけ小さくする必要があった。言語や環境によっては識別子の長さに制約があった。変数名や関数名を自動補完するIDEやエディタがあまり普及していなかった。プログラムが複数の開発者で共有されることが少なかった。(などと推測)

該当項目がゼロでも安心しない

該当項目がゼロだった人も安心してはいけません。
「10年以上前の技術の成功体験は実はあまり役立っていないどころか、綺麗なプログラムを書く邪魔をしている時すらある。」という意見もあります*8。
JavaやC#もほぼ10年選手です。つまり、JavaやC#の知識が徐々にレガシー化してきている可能性もあるわけです。

たとえば上の改善方法でforループをforeach文に変更する例を挙げましたが、最近のC#だったらLINQで処理する方法もありますし、Rubyならeachメソッドとブロックで処理する方法もあります。
こうした方法は10年前に存在しなかったり、あまり知られていなかったりする方法です。
JavaやC#を使いこなしているプログラマがレガシープログラマを笑っていても、ぼーっとしてたら10年後には自分がレガシープログラマと笑われている可能性もあります。

過去の成功体験に満足せず、これまでの成功体験をぶち壊しながらでも新しい技術を学んでいくのが真のモダンプログラマですよね？

C#プログラマのための理解度チェックリスト

前回のエントリでは「新しい言語を勉強したって、前に使ってた言語と同じような書き方をしてたら意味がない」という話をしました。
そこでC#を題材にして、C#を本質からマスターできているかどうかを確認するためのチェックリストを作ってみました。
これらの質問に対してすべて自分の言葉で説明できるのであれば、あなたはきっとC#をC#らしく使えているはずです。

interfaceって何のためにある？どういうときに使う？それがあったら何が嬉しい？
抽象クラスや抽象メソッドって何のためにある？どういうときに使う？それがあったら何が嬉しい？
virtualって何？なんでJavaにはvirtualがない？
名前空間って何？それがあったら何が嬉しい？
クラスって何？自分で新しくクラスを作る場合の注意点は何？(オブジェクト指向設計的な観点から)
クラスと構造体の違いって何？参照型と値型の違いって何？
フィールドやメソッドの可視性って何？可視性にはどんな種類がある？全部public宣言しちゃうとどんな問題が起きる？
staticを付けているメソッドや変数は付けないときと何が違ってくる？
newっていったい何してるんでしょう？
コンストラクタって何？普通のメソッドとどう違う？
thisっていったい何なんでしょう？
配列、リスト、ディクショナリの違いってそれぞれ何？どういう用途で使い分ける？
ジェネリッククラスって何？ジェネリッククラスがあると何が嬉しい？IListとIList(T)の違いは何？
下のコードはどうして左辺と右辺のクラスが違うの？

IList<string> hoge = new List<string>();

以下のようなコードの問題点はどこでしょう？(ヒント:例外の使い方)

public bool DoSomething()
{
  try
  {
    DoIt();
  }
  catch (Exception e)
  {
    Log(e.Message);
    return false;
  }
  return true;
}

「disposeパターン」「using文」「ガーベッジコレクタ」という言葉を織り交ぜて、C#におけるリソース管理について説明してください
ローカル変数、インスタンス変数、クラス変数の違いはそれぞれ何でしょう？
プロパティとメソッドの違いは何でしょう？
Javaでよく使われるgetter/setterメソッドとC#のプロパティの違いは何でしょう？
下のコードは一見するとどちらもキャストを行っているように見えるが、後者は実際には型変換を行っていないのはなぜ？

int i = 30;
short s = (short)i;

object obj = GetSomeObject();
MyClass myClass = (MyClass)obj;

下のコードは実行時にどのような違いが現れる？(ヒント:GetSomeTextMaybeNullというメソッド名からリスクを推測する)

object obj = GetSomeTextMaybeNull();
string s1 = obj.ToString();
string s2 = (string)obj;

文字列の連結は「+演算子」ではなく、StringBuilderを使えって言われる理由は何？
下のコードを実行したときに現れる違いは何？また、違いが現れるのはなぜ？「参照」という言葉を織り交ぜて説明してください。

string str = "Hello,";
AppendToString(str);
Console.WriteLine(str); // 文字列を表示

StringBuilder sb = new StringBuilder("Hello,");
AppendToStringBuilder(sb);
Console.WriteLine(sb.ToString()); // 文字列を表示

private void AppendToString(string str)
{
  str += "World!";
}

private void AppendToStringBuilder(StringBuilder sb)
{
  sb.Append("World!");
}

不変クラス(immutableなクラス)って何？
ステートレスなクラス、ステートフルなクラスってそれぞれ何？
深いコピー(Deep Copy)、浅いコピー(Shallow Copy)って何？
以下のようなメンバを宣言するときはアッパーキャメルケース(AbcXyz)、ローワーキャメルケース(abcXyz)のどちら？
- クラス
- メソッド
- プロパティ
- インスタンス変数
- メソッド引数
- ローカル変数

チェックリストは以上です。
回答例はここには挙げませんが、以下のような本を読むとおそらく大半の問題に回答できるはずです。

Effective C#

作者: ビルワグナー,Bill Wagner,鈴木幸敏
出版社/メーカー: 翔泳社
発売日: 2009/12/17
メディア: 大型本
購入: 5人クリック: 210回
この商品を含むブログ (14件) を見る

More Effective C#

作者: Bill Wagner,長尾高弘
出版社/メーカー: 翔泳社
発売日: 2009/12/01
メディア: 大型本
購入: 9人クリック: 140回
この商品を含むブログ (21件) を見る

.NETのクラスライブラリ設計 (Microsoft.net Development Series)

作者: Krzysztof Cwalina,Bard Abrams,藤原雄介
出版社/メーカー: 日経BP社
発売日: 2009/12/23
メディア: 単行本
購入: 10人クリック: 603回
この商品を含むブログ (34件) を見る

そして回答できるようになればきっと、あなたが以下のリンクにあるようなコラムを書くことはないと思います。。。

http://el.jibun.atmarkit.co.jp/minagawa/2010/04/post-ebc4.html

2010-11-10

モデルを中心としたソフトウェアアーキテクチャを採用する意義

システム開発 C#

今とある新規社内システムの設計と開発を担当しています。
ソフトウェアアーキテクチャはおいらが考えました。
簡単に言うとこんな感じです。

UI層/サービス層/データアクセス層の3レイヤーに分ける
開発言語はC#、UIはASP.NET、データアクセスはADO.NETを採用 (バージョンはいずれも2.0)
データのやり取りはデータベーススキーマから自動生成した型付けデータセットを使う

MVCで言うところのModelが型付けデータセットで、ViewとControllerがASP.NETです。
サービス層は基本的にこの型付けデータセットを出し入れします。
つまりテーブルの行単位でデータをやりとりします。

で、こういうスタイルのアーキテクチャに慣れていない開発者には、どうもこれが非効率で面倒なアーキテクチャに見えるようです。
まあ確かに言わんとすることは分からんでもありません。

例えば本のISBN、タイトル、出版社名、コメントをGridViewに一覧表示する必要があったとします。
単純に考えると下のようなSQLを発行してGridViewにバインドすればほぼ終わりです。

SELECT A.isbn, A.book_title, B.publisher_name, A.book_comment
FROM book A
INNER JOIN publisher B ON
B.publihser_id = A.publisher_id
WHERE
A.book_price < @book_price

サービスのAPIはこんな感じです。

public DataTable GetBookInfo(int price);

こう書けば画面に必要なデータがすべて取れるところを、型付けデータセットを使うとこんなクエリを発行する必要があります。

-- bookテーブルを取得(全カラム)
SELECT DISTINCT A.isbn, A.book_title, A.book_price, A.publisher_id, A.book_comment 
FROM book A
INNER JOIN publisher B ON
B.publihser_id = A.publisher_id
WHERE
A.book_price < @book_price

-- publisherテーブルを取得(全カラム)
SELECT DISTINCT B.publisher_id, B.publisher_name, B.publisher_location 
FROM book A
INNER JOIN publisher B ON
B.publihser_id = A.publisher_id
WHERE
A.book_price < @book_price

この場合のサービスのAPIはこんな感じです。

public BookStoreDataSet GetBookInfo(int price);

クエリを2回発行していますし、UIでは使わない列まで取得しています。
さらにこれをGridViewに表示する場合、テーブル(コード上はDataTable)間のリレーションを考慮してGridViewにバインドしなければならないため、UI層ではもうひと手間必要になります。

ところで今度はデータを更新する場合を考えます。
たとえばコメントを更新する場合はこんな感じになります。

型付けデータセットを使わない場合

// UI側のロジック
int isbn = this.GetIsbn();
string comment = this.GetComment();
this.service.UpdateBookComment(isbn, comment);

-- SQL
UPDATE book SET book_comment = @book_comment WHERE isbn = @isbn

型付けデータセットを使う場合

// UI側のロジック
bookStoreDataSet.book.isbn = this.GetIsbn();
bookStoreDataSet.book.book_comment = this.GetComment();
this.service.UpdateBook(bookStoreDataSet); // bookテーブルを更新する共通メソッド

-- SQL
-- 自動生成されたUPDATE文を使用するので省略

更新時は型付けデータセットの方が若干有利に見えます。
というのもデータを格納する入れ物(データセット)も更新用のロジック/SQLも自動生成されたものを再利用できるからです。

さて、おいらはなぜ一見メリットが少なそうに見える型付けデータセットを採用したのでしょうか？
カラム名やカラムのデータ型をコンパイラがチェックしてくれるので、コードの安全性が増すというのも大きな理由の一つですが、それ以外にも以下のような理由があります。

画面ごとに専用の取得ロジック、更新ロジックを用意していると個別最適にはなるが、結局何も再利用できない。また開発者間で実装のスタイルがバラけてしまい、他人のコードが理解しづらい。

型付けデータセットならどの開発者が実装してもソースコード上のデータ構造が一貫するので理解しやすい。また更新ロジックは自動生成されたものが使えるので、それを必要に応じて再利用できる。

画面は変わりやすい。個別最適なアーキテクチャの場合、画面で必要な情報が増えたり減ったりすると、その都度取得ロジックや更新ロジックを変更する必要がある。つまり、UI層だけでなく、サービス層やデータアクセス層の変更も必要になる。

画面に比べてテーブルスキーマは比較的安定している。サービス層は変化しやすいUI層ではなく、変化しにくいテーブルスキーマに依存することで、UIの変更(画面変更)によるインパクトを受けにくくなる。

たとえば、上の例で出版社の所在地(publisher_location)を追加表示する必要が出てきても、サービス層はすでにpublisherテーブルの全カラムを返却しているので変更の必要がない。つまりUI層の変更だけで改造が完了する。

まとめると、「テーブルスキーマをシステムのデータモデルと見なし、システム全体をモデルに依存する形で設計する。モデルは比較的安定しているので変更が発生しにくく、処理の再利用もしやすくなる」ということです。

こうした設計が変更に強く、保守性や拡張性の高いシステムの構築に繋がっていくわけです。

ただし、考慮点が全くないわけではありません。

まず、UI層は相対的に地位が低いため、UIはデータモデルと画面上の入出力項目をせっせこせっせこと変換する必要があります。よって、UI層のロジックは若干複雑化する可能性があります。

それからADO.NETの型付けデータセットに限って言うと、データの取得に関してはさすがに非効率な感が否めません。
テーブルごとに似たようなSELECT文を何度も発行するのはハッキリ言ってイケてないです。
画面ごとに開発者がSELECT文を作らなければいけない点もいただけません。
できればADO.NETではなく、オープンソースのO/Rマッピングツール等を活用して、SELECT文も自動生成できるようにした方が良いと思います。
(詳しくは知りませんが、最近のADO.NETでリリースされたEntity Frameworkなんかを使うとこのへんの問題が解決するのでしょうか？)

本当に小規模なシステムであれば個別最適なアーキテクチャでもさほど変わりはないかもしれませんが、システムが大きくなればなるほど、個別最適の観点からはNGでも保守性や拡張性を考慮した全体最適なアーキテクチャ(= モデルを中心としたアーキテクチャ)を採用するメリットが大きくなります。

・・・とまあ、こんな感じです。
ここまで長々と説明してきましたが、う〜んなかなか難しいですね。
メリットはあるはずなのに、どうもそのメリットを強調するような文章にならないのがもどかしいです。
このあたりをうまく説明できないと、レガシーなスタイルに慣れきっている人たちから「個別最適なアーキテクチャ」へ引き戻そうとするプレッシャーを掛けられるんですよね〜。
まだまだ自分の中でも完全に消化しきれていない部分があると思うので、今後も要精進っちゅーことですね。

なお、この話に関連するネタが以下のエントリにありますので、興味がある方はご参考までにどうぞ。

犬小屋と高層ビルの違い - give IT a try