人のコードを扱ってると「あ、これはちょっとあかんコードだ」と気づく時がある。設計かコーディングかにこだわらず書き連ねていく。実は過去に自分がやらかした例もある。
できるだけ簡潔な箇条書きになることを目指す。それぞれ単発の記事にするには短いのでちょいちょい更新していく。
この記事でアプリケーションコードとはライブラリやフレームワークを使う側のコードを指し、ライブラリコードとはライブラリやフレームワークを提供する側のコードを指している。
STLのコンテナを使わず自分でリストなどのデータ構造を実装している
設計の領域。単に連結リストが使いたいという動機で自分で実装すべきではない。
実装・メンテナンスのコストが得られるパフォーマンスに見合わない。
加えてコードを引き継いだ者が非標準のデータ構造のインターフェースと実装を理解するコストが発生する。
参照型がローカルに宣言されている、参照型を変数に別名をつけるのために使用している
参照型はライフタイムが尽きたインスタンスを参照する可能性がある。
std::string get_some_string()
{
// 簡単な例
std::string& head = long_long_name_string_vector[0]; // 長いし名前を付け替えておこう
/*
いろいろな処理
*/
long_long_name_string_vector.clear(); // さよなら long_long_name_string_vector[0]
return head; // アウト。headの参照先はすでに存在しない。
}
基本的に参照型は（lvalue reference、rvalue referenceともに）関数のインターフェース（引数、戻り値）においてのみ現れるべきである。少なくともアプリケーションコードでローカルに参照型を宣言する合理的なシチュエーションはない。
アプリケーションコードでnewが使用されている
メモリの確保はクラスを提供する側が行うべきである。
アプリケーションコードのクラスがインスタンスを保持したい場合、実体で保持するか、std::make_sharedを使用する。
ローカル変数の宣言が関数の冒頭のみで行われている
実装の問題。Cの癖が抜けていない。コードの読みやすさの観点と（一応）速度の観点から害悪。
変数は必要になった時に必要なスコープの中で宣言すべきである。
void func(int a, int b)
{
int i; // iの説明
int foo; // fooの説明
int bar; // barの説明
/*
...
*/
foo = sub_func(); // 「fooの型ってなんだったっけ・・・？」「そもそもfooはなんだっけ・・・？」
for(i = 0; i < 10; ++i)
{
bar = sub_func2();
// 「barはもういらないんだけど宣言どこだ？」「forの外で使われていないか？」
// 「値を上書きして大丈夫なのか？」
}
/*
...
*/
}
メンバ変数という名前のクラス内グローバル変数
気が向いたら書く
メンバ関数を実行した結果を別のメンバ関数で取り出させている。
設計の問題。
ObjectDetector detector;
detector.detect(input);
Objects objects = detector.getResult();
detector が状態を持つことになり detect() がconstでなくなる。したがって ObjectDetector のconst参照渡しができなくなる。またマルチスレッド化がめんどくさくなる。
クラスに状態を持たせるのはデータを表すクラス、またはリソースだけに止める。
っつーかそもそも計算だけならクラスじゃなくて関数でいいじゃん、という話ではある。
オブジェクトをvoid*で渡す
result_t some_function(void* p_Obj)
{
some_class* sc_p = (some_class*)p_Obj;
// ...
return sc_p->some_action();
}
意味わからん。