When one adopts the strategy of forming one's own mental map of a problem domain and attempting to simplify it, one faces the problem of when to stop working on the map. This applies at every level of design. The extraordinary thing is that there almost always is a deep solution, that is significantly simpler than anything else and manifestly minimal. (There may be more than one way of expressing it, but then the relationship will be manifest.) Although homilies like `You'll know it when you see it!' are undoubtedly true, they don't tell one where to look.
As the only honest argument, we can offer here is the promise that this really happens. And although it proves nothing, all we can do is show you a worked example reduced to a minimal state. But it does work - ask anyone who's tried.
The example we will present is from Jeffrey Richter's excellent Advanced Windows. This book is essential reading for anyone intending to program against Microsoft's Win32 Application Programming Interface (API) (because otherwise, you won't have a mental map of the system semantics).
Richter sets out to provide as clear an exposition of how to use Win32 as he can, but even in his examples (and partially as a result of the conventions he is following), complexity that we can lose appears. On page 319, there is a function SecondThread() We'll just look at the function, and leave the remainder of the program and some global definitions:
```cpp
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():
}
// Release the mutex.
ReleaseMutex(g_hMutex);
} else {
// The mutex was abandoned.
break;// Exit the while loop.
}
}
return(0);
}
```
First let's just simplify the brace style, lose the extra space between the keyword and open bracket, and the redundant ReleaseMutex comment. We are aware that there is a religious war between the followers of K&R and Wirth on brace style, but getting the blocking symmetric really does make things easier to see. The extra line it takes will be won back later - bear with us!
```cpp
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);
}
```
It's easy to lose one local variable: dw is assigned then tested in the next statement. Inverting the sense of the test helps locality of reference (testing then changing g_nIndex). And while we are about it there is no point incrementing g_nIndex just to subtract 1 from its current value in the next operation! We are already using the C post-increment operator, which was provided for just this sort of job.
```cpp
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);
}
```
The break depends only on the result of WaitForSingleObject, so it is a simple matter to move the test up into the controlling expression, eliminating both the break and a level of indentation:
```cpp
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);
}
```
Now just squeeze... We know that lots of coding standards say that we must always put the curly brackets in because sometimes silly people made unreadable messes, but look what happens when we dump the rule and concentrate on the intent of making the code readable.
```cpp
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);
}
```
Now for some real heresy. Gosh, by the time we've finished this total irresponsibility the result will be totally illegible. (Or of course, common sense can do more good than rules.)
Heresies are, if we know what our variables are, we'll know their type. If we don't know what a variable is for, knowing its type won't help much. Anyway, the compilers type-check every which way these days. So drop the Hungarian and gratuitous fake type extensions that are just #defined to nothing somewhere. Hiding dereferences in typedefs is another pointless exercise because although it accomplishes a kind of encapsulation of currency, it is never sufficiently contingent that we never have to worry about it, and then a careful programmer has to keep the real types in mind. Maintaining a concept of long pointers in variable names in what is a flat 32 bit API is pretty silly too.
```cpp
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);
}
```
Now watch. We will hit the Quality Plateau...
```cpp
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);
}
```
Eleven lines vs 26. One less level of indentation, but the structure completely transparent. Two local variables eliminated. No else clauses. Absolutely no nested elses. Fewer places for bugs to hide.
Finally, the text has made it clear that different threads execute functions in different contexts. It is not necessary to define one function called FirstThread(), with exactly the same cut and paste definition as SecondThread(), and call them,
hThreads[0] = CreateThread(..., FirstThread, ...);
hThreads[1] = CreateThread(..., SecondThread, ...);
When it could just say,
hThreads[0] = CreateThread(..., TheThread, ...);
hThreads[1] = CreateThread(..., TheThread, ...);
About a third of this example is actually cloning code! If we did see a bug in one instance, we'd have to remember to correct the other one too. Why bother when we can just junk it. It's this kind of thing that stresses deadlines.
Copyright (c) Alan G Carter and Colston Sanger 1997
Top comments (0)