CodeSOD: Whiling Away the Time

There are two ways of accumulating experience in our profession. One is to spend many years accumulating and mastering new skills to broaden your skill set and ability to solve more and more complex problems. The other is to repeat the same year of experience over and over until you have one year of experience n times.

Anon took the former path and slowly built up his skills, adding to his repertoire with each new experience and assignment. At his third job, he encountered The Man, who took the latter path.

If you wanted to execute a block of code once, you have several options. You could just put the code in-line. You could put it in a function and call said function. You could even put it in a do { ... } while (false); construct. The Man would do as below because it makes it easier and less error prone to comment out a block of code:

 Boolean flag = true; while (flag) { flag = false; // code> break; }

The Man not only built his own logging framework (because you can't trust the ones out there), but he demanded that every. single. function. begin and end with:

 Log.methodEntry("methodName"); ... Log.methodExit("methodName");

...because in a multi-threaded environment, that won't flood the logs with all sorts of confusing and mostly useless log statements. Also, he would routinely use this construct in places where the logging system had not yet been initialized, so any logged errors went the way of the bit-bucket.

Every single method was encapsulated in its own try-catch-finally block. The catch block would merely log the error and continue as though the method was successful, returning null or zero on error conditions. The intent was to keep the application from ever crashing. There was no concept of rolling the error up to a place where it could be properly handled.

His concept of encapsulation was to wrap not just each object, but virtually every line of code, including declarations, in a region tag.

To give you a taste of what Anon had to deal with, the following is a procedure of The Man's:

 #region Protected methods protected override Boolean ParseMessage(String strRemainingMessage) { Log.LogEntry(); # region Local variables Boolean bParseSuccess = false; String[] strFields = null; # endregion //Local variables # region try-cache-finally [op: SIC] # region try try { # region Flag to only loop once Boolean bLoop = true; # endregion //Flag to only loop once # region Loop to parse the message while (bLoop) { # region Make sure we only loop once bLoop = false; # endregion //Make sure we only loop once # region parse the message bParseSuccess = base.ParseMessage(strRemainingMessage); # endregion //parse the message # region break the loop break; # endregion //break the loop } # endregion //Loop to parse the message } # endregion //try # region cache // [op: SIC] catch (Exception ex) { Log.Error(ex.Message); } # endregion //cache [op: SIC] # region finally finally { if (null != strFields) { strFields = null; // op: why set local var to null? } } # endregion //finally # endregion //try-cache-finally [op: SIC] Log.LogExit(); return bParseSuccess; } #endregion //Protected methods

The corrected version:

 // Since the ParseMessage method has it's own try-cache // on "Exception", it will never throw any exceptions // and logging entry and exit of a method doesn't seem // to bring us any value since it's always disabled. // I'm not even sure if we have a way to enable it // during runtime without recompiling and installing // the application... protected override Boolean ParseMessage(String remainingMessage){ return base.ParseMessage(remainingMessage); }

[Advertisement] Otter, ProGet, BuildMaster - robust, powerful, scalable, and reliable additions to your existing DevOps toolchain.