(toppers-users 521) Re: TOPPERS/JSP のコンフィグレータ

[Takagi, Yusei]

こんにちは、高木です。

「delete this;」ですが、ざっと検索したところ、Directory::_eraseで行って
いるようでした。
この場合、わざわざ delete this;を使わなくても、_eraseで行っている処理を
デストラクタで
行えば済むのではないでしょうか？そして、_eraseを呼び出す代わりにdeleteし
てやれば、
普通のコードになると思います。
std::map<>::eraseは不正な反復子を渡さない限り例外を出すことはありません
ので、
デストラクタから例外がでることはありません。

ついでなので、これまでに見つけた他の問題点についても報告しておきます。

1. new演算子を多重定義しているにもかかわらず、対になるdelete演算子を定義
していない。
　　　　再定義されたnew演算子は内部でmallocを呼び出していますが、処理系
がデフォルト
　　　　で提供しているnew演算子がmallocを使っているかどうかは実装依存で
す。
　　　　それに対して::operator deleteを呼び出してしまうと間違いなくク
ラッシュします。

2. ヘッダファイルの最後にある#endifの後ろに改行がない。
　　　　それを許可する処理系の拡張機能に依存しています。

3. デストラクタから例外を出さないためには、自分でthrowしないだけでなく、
例外を出すかも
　しれない処理を排除する必要がある。
　　　　一般に例外を出さないことがはっきりしているのは、組み込み型の操
作、明示的にthrow()
　　　　が付けられている関数の呼び出し、仕様上例外を出さないことが保証さ
れている関数の
　　　　呼び出し（例えばCから受け継いだ標準関数）だけです。

4. fopenの第２引数に"rt"という処理系の独自拡張が使われている。
　　　　テキストモードの場合"t"は不要です。

5. 予約識別子が使われている。
　　　　アンダスコアで始まりアンダスコアか大文字が続く識別子は予約されて
います。また、
　　　　二重のアンダスコアを含む識別子も予約されています。
　　　　アンダスコアで始まり、小文字や数字が続く識別子はファイルスコープ
でのみ予約され
　　　　ていますが、それにはマクロ定義も含まれれるので結局使えません。
　　　　移植性に配慮する場合は予約識別子は全廃すべきです。

6. Exceptionクラスは多重例外を出す要因が多数含まれる。
　　　　例外をこれから throwしようとするときに、さらに例外を発生する可能
性がある処理を行っ
　　　　てはいけません。例えば、std::stringのコンストラクタは更なる例外
を引き起こす可能性
　　　　を持つ代表格です。実際の throw時にさらにnewするのは最悪です。

7. 例外をポインタで受けている。
　　　　例外をポインタで受けると、そのポインタが指すオブジェクトの所有権
が不明確になりがち
　　　　です。かといって、値で受けると効率の低下やスライスの問題が発生し
ます。例外をcatch
　　　　するのは参照で行うのが最適です。

今のところ、必ず改善すべき内容はこんなところです。
他に気になる点としては、

8. void*が多用されている。
9. 変換コンストラクタや変換関数、型違いの代入演算子が多用されている。
　　　　これらはコンパイラの型チェック機能を無効にしてしまうので危険で
す。特にユーザがカスタ
　　　　マイズしようとしたときに思わぬところでバグを作ってしまいます。

10. 反復子のインクリメントに後置形の++を使用している。
　　　　クラス型に対する++は、どちらでもよい場合は前置形を使うべきです。
後置形を使うと効率
　　　　の低下だけでなく、一時オブジェクト構築時に例外が出る危険性を伴い
ます。

11. 仮想デストラクタを持たないクラスからpublic継承している。
　　　　これもユーザがカスタマイズしようとしたときに思わぬところでバグを
作る原因になります。

もし時間がとれるようでしたら、「Exceptional C++」という本を読んでみてく
ださい。すごく役に立つ
と思います。

以上、言いたい放題でしたが、よりよいものを作っていただきたいと期待してお
りますので、どうか
ご容赦ください。

---------
高木悠成