25 November 2011

Горбатого могила исправит

Давно слежу за творчеством Джона, нашего, Кармака.
Он несомненный гений, мощный математик и алгоритмист. Но при этом -- редкостный гавнокодер. 



На старости лет Джон решил наконец-то выучить что-то сложнее Си, взявшись за C++. Первый серьезный плюсовый проект Джона -- Doom 3, сырцы которого были выложены на этой неделе в публичный доступ. Я, повизгивая от злорадства, тут же кинулся их качать, дабы убедиться в своих прогнозах -- миру открылась очередная порция говнокода. 

Так оно и оказалось. 

Код -- классический "си с классами". Правда, есть немного шаблонов -- свой велосипед с контейнерами вместо STL. 
Функции на сотни строк в порядке вещей. При этом даже переменные (даже для циклов!) описываются в начале функции, как в си стандарта 80-х годов (см. пример #1). Описанные в начале функции два десятка непроинициализированных локальных переменных -- абсолютно типичная картина в коде. 
Что такое список инициализации в конструкторе Джон не знает. 
Функции printf() like понатыканы повсюду. Это же так удобно! 
Что такое виртуальные функции Джон знает, но и dynamic_cast ему лучший друг, если чо. 

Нет исключений. Ладно, это еще можно понять. Но почему бы не использовать пространства имен, блин? Почему бы не использовать умные указатели для совсем уж очевидных вещей, на хрена тыкать этот ручной delete? (см. пример #2)

Ну и понятно, что такими вещами, как сериализация через шаблон, когда код можно написать один раз, в коде и не пахнет. 

И при этом Джон сетует, что когда он видит хороший современный плюсовый код, то эта вся история "all STL this, boost that, fill-up-the-property list, dispatch the event, and delegate that" вызывает в нем только одно чувство -- как это неэффективно и медленно! 

Ну да, ну да.

В общем, математику можете смотреть у Джона, но упаси вас б-г кодить в таком стиле! 

Пример #1. 

Копи-паст таких вот функций, вместо сериализации через шаблон. 
Работа с переменными -- за гранью добра и зла.
Бонус: совершенно неоправданное использование препроцессора. 

void idAI::Save( idSaveGame *savefile ) const 
{
int i;

savefile->WriteInt( travelFlags );
savefile->WriteFloat( kickForce );
savefile->WriteBool( ignore_obstacles );

// и еще 50 строк кода

savefile->WriteInt( missileLaunchOffset.Num() );
for( i = 0; i < missileLaunchOffset.Num(); i++ ) {
savefile->WriteVec3( missileLaunchOffset[ i ] );
}

savefile->WriteString( projectileName );
savefile->WriteFloat( projectileRadius );

// и еще 50 строк кода

savefile->WriteInt( lookJoints.Num() );
for( i = 0; i < lookJoints.Num(); i++ ) {
savefile->WriteJoint( lookJoints[ i ] );
savefile->WriteAngles( lookJointAngles[ i ] );
}

savefile->WriteFloat( shrivel_rate );

// и еще 50 строк кода

#ifdef _D3XP
savefile->WriteInt(funcEmitters.Num());
for(int i = 0; i < funcEmitters.Num(); i++) {
funcEmitter_t* emitter = funcEmitters.GetIndex(i);
savefile->WriteString(emitter->name);
savefile->WriteJoint(emitter->joint);
savefile->WriteObject(emitter->particle);
}

harvestEnt.Save( savefile);
#endif

Пример #2. 

Твою мать! В функции 7 (!) раз пишется "delete src". 
Умные указатели для слабаков, настоящие ковбои пишут такой вот эффективный и быстрый код, вооружившись тулзами для поиска утечек памяти!

bool idCollisionModelManagerLocal::LoadCollisionModelFile( const char *name, unsigned int mapFileCRC ) {
idStr fileName;
idToken token;
idLexer *src;
unsigned int crc;

// load it
fileName = name;
fileName.SetFileExtension( CM_FILE_EXT );
src = new idLexer( fileName );
src->SetFlags( LEXFL_NOSTRINGCONCAT | LEXFL_NODOLLARPRECOMPILE );
if ( !src->IsLoaded() ) {
delete src;
return false;
}

if ( !src->ExpectTokenString( CM_FILEID ) ) {
common->Warning( "%s is not an CM file.", fileName.c_str() );
delete src;
return false;
}

if ( !src->ReadToken( &token ) || token != CM_FILEVERSION ) {
common->Warning( "%s has version %s instead of %s", fileName.c_str(), token.c_str(), CM_FILEVERSION );
delete src;
return false;
}

if ( !src->ExpectTokenType( TT_NUMBER, TT_INTEGER, &token ) ) {
common->Warning( "%s has no map file CRC", fileName.c_str() );
delete src;
return false;
}

crc = token.GetUnsignedLongValue();
if ( mapFileCRC && crc != mapFileCRC ) {
common->Printf( "%s is out of date\n", fileName.c_str() );
delete src;
return false;
}

// parse the file
while ( 1 ) {
if ( !src->ReadToken( &token ) ) {
break;
}

if ( token == "collisionModel" ) {
if ( !ParseCollisionModel( src ) ) {
delete src;
return false;
}
continue;
}

src->Error( "idCollisionModelManagerLocal::LoadCollisionModelFile: bad token \"%s\"", token.c_str() );
}

delete src;

return true;


Проблема с удалением решается даже в рамках Си элементарным рефакторингом -- выделением функции, работающей с src.

Такого рода код вызывает у меня неприкрытое отвращение. Именно поэтому я никогда в жизни не буду по доброй воле кодить на языках без RAII или сборки мусора. 

No comments:

Post a Comment