The Programmers' Stone (Программистский камень)

Carter Alan

dw = WaitForSingleObject(g_hMutex, INFINITE);

if (dw == WAIT_OBJECT_0) { // Mutex became signalled.

if (g_nIndex >= MAX_TIMES) {

fDone = TRUE;

} else {

g_nIndex++;

g_dwTimes[g_nIndex - 1] = GetTickCount:

} // Release the mutex.

ReleaseMutex(g_hMutex);

} else { // The mutex was abandoned.

break; // Exit the while loop.

}

}

return(0);

}

Для начала просто упростим стиль скобок, уберем пробел между ключевым словом и открывающей скобкой, а также многословный комментарий к ReleaseMutex. Мы в курсе, что идет религиозная война между последователями Кернигана и Ритчи (K&R) и последователями Вирта (Wirth)

по поводу стиля скобок, но симметрия обрамления блока действительно позволяет лучше увидеть некоторые вещи. Дополнительная строка, которая при этом появляется, даст выигрыш чуть позднее -- следуйте за нами!

DWORD WINAPI SecondThread(LPVOID lpwThreadParm) { BOOL fDone = FALSE; DWORD dw; while(!fDone) { // Wait forever for the mutex to become signaled. dw = WaitForSingleObject(g_hMutex, INFINITE); if(dw == WAIT_OBJECT_0) { // Mutex became signalled. if(g_nIndex >= MAX_TIMES) { fDone = TRUE; } else { g_nIndex++; g_dwTimes[g_nIndex - 1] = GetTickCount: } ReleaseMutex(g_hMutex); } else { // The mutex was abandoned. break; // Exit the while loop. } } return(0); }

Очень легко можно избавиться от одной локальной переменной: dw присваивают значение, а в следующей операции тестируют. Инвертирование смысла проверки помогает локализовать ссылку (проверка, затем изменение g_nIndex). А пока мы здесь, нет смысла инкрементировать g_nIndex просто для того, чтобы вычесть 1 из текущего значения в следующей операции! Мы уже использовали постфиксную форму оператора инкремента языка Cи, который как раз для этого и предназначен.

DWORD WINAPI SecondThread (LPVOID lpwThreadParm) { BOOL fDone = FALSE; while (!fDone) { // Wait forever for the mutex to become signaled. if (WaitForSingleObject(g_hMutex, INFINITE)==WAIT_OBJECT_0) { // Mutex became signalled. if (g_nIndex < MAX_TIMES) { g_dwTimes[g_nIndex++] = GetTickCount; } else { fDone = TRUE; } ReleaseMutex(g_hMutex); } else { // The mutex was abandoned. break;// Exit the while loop. } } return(0); }

Прерывание цикла (break) зависит только от результата WaitForSingleObject, поэтому естественно переместить проверку в управляющее выражение, избавляясь от прерывания цикла и одного уровня вложенности:

DWORD WINAPI SecondThread (LPVOID lpwThreadParm) { BOOL fDone = FALSE; while (!fDone && WaitForSingleObject(g_hMutex, INFINITE)==WAIT_OBJECT_0) { // Mutex became signalled. if (g_nIndex < MAX_TIMES) { g_dwTimes[g_nIndex++] = GetTickCount; } else { fDone = TRUE; } ReleaseMutex(g_hMutex); } return(0); }

Теперь просто сожмем... Мы знаем - многие стандарты кодирования говорят, что мы всегда должны ставить фигурные скобки, поскольку иногда у глупых людей получается нечитаемая мешанина, но посмотрите, что получается, когда мы пренебрегаем этим правилом и концентрируемся на повышении читаемости кода.

DWORD WINAPI SecondThread (LPVOID lpwThreadParm) { BOOL fDone = FALSE; while (!fDone && WaitForSingleObject(g_hMutex, INFINITE)==WAIT_OBJECT_0) { if (g_nIndex < MAX_TIMES) g_dwTimes[g_nIndex++] = GetTickCount; else fDone = TRUE; ReleaseMutex(g_hMutex); } return(0); }

Теперь немного настоящей ереси. Черт возьми, в момент когда мы покончим с этой полной безответственностью, результат окажется совершенно неочевидным. (Здравый смысл поможет сделать лучше, чем правила.)

Ересь в том, что если мы знаем, для чего наши переменные, то мы знаем их типы. Если мы не знаем, для чего предназначена переменная, знание ее типа мало поможет. В любом случае, компилятор все равно сделает проверку типов. Поэтому избавимся от венгерской записи, а заодно и от переопределений типов, которые просто определены ( #define ), но не для нас. Сокрытие разыменования используя typedef - другое бесцельное упражнение, поскольку хотя и позволяет выполнить некоторую инкапсуляцию валюты, этого совершенно

недостаточно, чтобы избавиться от беспокойства по этому поводу, поэтому аккуратные программисты вынуждены держать настоящие типы в голове. Поддержка концепции дальних указателей в именах переменных для 32 битного API с плоской адресацией -- тоже довольно глупое занятие.

DWORD SecondThread (void *ThreadParm) { BOOL done = FALSE; while (!done && WaitForSingleObject(Mutex, INFINITE)==WAIT_OBJECT_0) { if (Index < MAX_TIMES) Times[Index++] = GetTickCount; else done = TRUE; ReleaseMutex(Mutex); } return(0); }

Теперь смотрите. Мы достигнем Плато Качества...

DWORD SecondThread(void *ThreadParm) { while(Index < MAX_TIMES && WaitForSingleObject(Mutex, INFINITE) == WAIT_OBJECT_0) { if (Index < MAX_TIMES) Times[Index++] = GetTickCount: ReleaseMutex(Mutex); } return(0); }

Одиннадцать строк против 26. На один уровень меньшая вложенность, но структура полностью прозрачна. Две локальных переменных исчезли. Нет блоков. Совсем нет вложенных else. Меньше мест, где могут скрываться ошибки.

(Если вы еще не программировали используя потоки (threads), то повторная проверка значения Index внутри тела цикла кажется грубой и ненужной. Если же программировали, то повторная проверка естественна и очевидна. Это очень важно: пока текущий поток приостановлен в WaitForSingleObject, другой поток скорее всего будет активен и изменит значение. То, что для вас очевидно, зависит от вашего опыта: еще одна мораль из этого примера -- рассмотрения только структуры куска кода недостаточно.)

Наконец, текст делает совершенно ясным, что разные потоки выполняют функции в разных контекстах. Поэтому совершенно не нужно определять функцию с именем FirstThread, в точности такую же, как SecondThread, и вызывать их так:

hThreads[0] = CreateThread(..., FirstThread, ...); hThreads[1] = CreateThread(..., SecondThread, ...);

Когда можно просто

hThreads[0] = CreateThread(..., TheThread, ...); hThreads[1] = CreateThread(..., TheThread, ...);

Почти треть этого примера получена клонированием! Если мы обнаружим ошибку в одной реализации, нам нужно будет не забыть исправить аналогичные ошибки везде. Зачем беспокоиться, когда можно просто слить их в одну. Это хороший способ, когда поджимают сроки.

Знание, а не число строк кода (KLOCS)

Программисты дороги. Результаты их работы должны быть собраны и использованы к выгоде их организации. Проблема в том, что традиционный способ паковщика подбить результаты -- подсчитать то, что они могут увидеть. Результаты изучения проблемы командой программистов, пришедшей к пониманию и проверившей это понимание, получив надежный код, выражаются не в числе строк кода (KLOCS), которые они набрали во время изучения. Результаты в окончательном понимании, к которому они пришли, когда закончили.

Причина, по которой важно оценивать результат таким образом, в том, что понимание показывает гораздо более простой способ реализации, чем тот, с которого команда начинала. Классическое поле сражения картостроителей с паковщиками в программировании состоит в том, что картостроители видят, что с тем, что они узнали, повторная реализация может быть сделана быстрее и не будет страдать от проблем сопровождения, вырисовывающихся в существующем коде. Паковщики видят, что картостроители безумствуют, пытаясь разгромить всю их работу (как будто нет резервной копии) и повторить работу нескольких последних месяцев, которые были ужасны, поскольку они, очевидно, не знали, что они делали (они хранят изменяющиеся вещи). Паковщики настраивают одного из своих защитников остановить картостроителей, и организация вынуждена забыть о достигнутом понимании, которое не может быть использовано в контексте существующего кода.