CodeSOD: A Well Mapped Error

Marvin's company had a problem. Their C++ application tended to throw out a lot of error codes. That was actually okay- that application had a lot of possible failure modes that it couldn't do anything about other than cough up an error and quit.

The problem was that their customers weren't particularly happy with messages like: Error: QB57, since it told them absolutely nothing. "RTFM" was not an acceptable response, so someone had to add some functionality to turn those cryptic error codes into meaningful messages.

Since it was a simple job, they handed it off to the co-worker who nobody really thought all that much of. He was a terrible programmer, but how badly could he screw a simple map/hash/dictionary problem up?

First, construct the data structures to hold the error messages:

ERROR_REC Error_A_List[] = { {"AA01", "Error AA01 Description"}, {"AA02", "Error AA02 Description"}, /* about 100 more! */};ERROR_REC Error_B_List[] = { {"BA01", "Error BA01 Description"}, {"BA02", "Error BA02 Description"},/* more of course */}/* etc for all the letters! - 26 total 2D arrays*/

Well, that's a bad start. Two-Dimensional arrays aren't the best way to represent a map, but hey, maybe they have a clever way of searching them?

const char* GetErrorDescription(char *wk_code) { bool done = false; int indx = 0, _wk_arr_size = 0; memset((char*) ret_desc, 0, 400); switch(wk_code[0]) { case 'A': _wk_arr_size = sizeof(Error_A_List)/sizeof(ERROR_REC); break; case 'B': _wk_arr_size = sizeof(Error_B_List)/sizeof(ERROR_REC); break;/* etc for C through Z - who doesn't love a 26-option case statement? */} ERROR_REC *wk_Error_List = new ERROR_REC[_wk_arr_size]; delete [] wk_Error_List; switch(wk_code[0]) { case 'A': wk_Error_List = Error_A_List; break; case 'B': wk_Error_List = Error_B_List; break;/* Second verse, same as the first"*/ } while (!done) { if (strcmp(wk_code, wk_Error_List[indx].Code) == 0) { done = true; strcpy(ret_desc, wk_Error_List[indx].Desc); } indx++; if (indx >= _wk_arr_size) done = true; else { if (strcmp(wk_code, wk_ResultCode_List[indx].Code) < 0) done = true; } } return ret_desc;}

" I guess not. Not only do we have a total of 62 branches worth of case statements, but we have a first-year CS major's solution to breaking out of a while loop. A loop that just brute forces its way through the list of possible codes.

The best thing we can say is that the code has been optimized- never will it need to brute-force its way through every possible error code. Instead, it brute-forces its way through approximately 1/26th of them.

Marvin said, "I was going to replace it with about 5 lines of code, but I think I'll get more enjoyment by leaving it the way it is." From this, I assume Marvin is a horrible person, or his company uses lines-of-code as a productivity metric.

[Advertisement] Manage IT infrastructure as code across all environments with Puppet. Puppet Enterprise now offers more control and insight, with role-based access control, activity logging and all-new Puppet Apps. Start your free trial today!