CodeSOD: Put a Dent in Your Logfiles

Valencia made a few contributions to a large C++ project run by Harvey. Specifically, there were some pass-by-value uses of a large data structure, and changing those to pass-by-reference fixed a number of performance problems, especially on certain compilers.

It's a simple typo," Valencia thought. Anyone could have done that." But they kept digging...

The original code-base was indented with spaces, but Harvey just used tabs. That was a mild annoyance, but Harvey used a lot of tabs, as his code style was nest as many blocks as deeply as possible". In addition to loads of magic numbers that should be enums, Harvey also had a stance that never use an int type when you can store your number as a double".

Then, for example, what if you have a char and you want to turn the char into a string? Do you just use the std::string() constructor that accepts a char parameter? Not if you're Harvey!

std::string ToString(char c){ std::stringstream ss; std::string out = ""; ss << c; ss >> out; return out;}

What if you wanted to cache some data in memory? A map would be a good place to start. How many times do you want to access a single key while updating a cache entry? How does four times" work for you? It works for Harvey!

void WriteCache(std::string key, std::string value){ Setting setting = mvCache["cache_"+key]; if (!setting.initialized) { setting.initialized=true; setting.data = ""; mvCache.insert(std::map<std::string,Cache>::value_type("cache_"+key,setting)); mvCache["cache_"+key]=setting; } setting.data = value; mvCache["cache_"+key]=setting;}

And I don't know exactly what they are trying to communicate with the mv prefix, but people have invented all sorts of horrible ways to abuse Hungarian notation. Fortunately, Valencia clarifies: Harvey used an incorrect Hungarian notation prefix while they were at it."

That's the easy stuff. Ugly, bad code, sure, but nothing that leaves you staring, stunned into speechlessness.

Let's say you added a lot of logging messages, and you wanted to control how many logging messages appeared. You've heard of logging levels", and that gives you an inspiration for how to solve this problem:

bool LogLess(int iMaxLevel){ int verboseLevel = rand() % 1000; if (verboseLevel < iMaxLevel) return true; return false;}//how it's used:if(LogLess(500)) log.debug("I appear half of the time");

Normally, I'd point out something about how they don't need to return true or return false when they could just return the boolean expression, but what'd be the point? They've created probabilistic log levels. It's certainly one way to solve the too many log messages" problem: just randomly throw some of them away.

Valencia gives us a happy ending:

Needless to say, this has since been rewritten... the end result builds faster, uses less memory and is several orders of magnitude faster.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.