今のプロジェクト、毎週コードチェックの日があります。
他の人がチェックインしたファイルを読んで、ここは間違ってるとかここは変じゃない?ということをお互いに指摘し合います。
自分は下っ端なのでチェックすることはあまり無いのですが、それでも時々はチェックしています。
自分は現在のアプリケーションの仕様についてはあまり理解していないので、設計的な指摘よりもミクロな部分ばかり指摘しています。
今まで指摘してきたのは、例えばこんな感じです。
1.
std::auto_ptr<char> pBuffer(new char[BUFFER_SIZE]);
char は POD 型なので問題ないですが、std::auto_ptr は delete で解放するので delete[] で解放する必要のあるポインタに対して使ってはいけないと思います。
2.
(progress > total) ? total : progress
std::min を使った方が分かりやすいと思います。
3.
if (m_pHoge)
{
delete m_pHoge;
m_pHoge = NULL;
}
null ポインタに対する delete は、C++ 規格上は安全です。
よって、(よっぽどひどい operator delete を定義していない限りは)null チェックをする必要は無いと思います。
4.
std::string str = ...;
DWORD ret = GetFileAttributes(str.c_str());
str は std::string なので、GetFileAttributesA を使用する必要があると思います。
5.
CString str = ...;
TCHAR dest[MAX_LENGTH] = { 0 };
_tcsncpy(dest, str, _countof(dest));
str.GetLength() が _countof(dest) 以上だった場合、dest には null 文字がコピーされず(_tcsncpy の仕様)問題が発生すると思います。
6.
void ErrorMsg(const TCHAR* sz, const char* file, int line)
{
TCHAR msg[1024];
_stprintf_s(msg, _countof(msg), _T("%s[%d] : %s"), file, line, sz);
::MessageBox(NULL, msg, NULL, MB_OK);
}
1個目の %s は %hs を指定する必要があります。
コードチェックは、する側もされる側も良い勉強になりますし、未然にバグを防げることもあります(というかこれがメインなのですが)。
また、他人に見られるという意識を持つことによって、書く側もそれなりに気を使って書いてくれるようになります。
他人のコードを読んでけちを付ける指摘するのはなかなか面白いものがあります。
是非試して頂ければと思います。