DevDisasters
The Long and Short of It
Not just once, but repeated in various lucrative apps -- a humongous chunk of code that should've been reduced to a mere line.
Thomas B.'s employer isn't a company that offers only a single flagship application -- it has dozens!
For years, the company has been making money year after year by developing and maintaining niche applications aimed at specific industries. Basically, it finds an existing application that's lacking a feature that might only be used by a handful of groups in the world and then implements it.
However, despite being lucrative, it's a somewhat chaotic business model, occasionally leading to a few support nightmares. Case in point: One of the company's C# application libraries processes the images inside a PDF and sends them through an automatic color correction library with perhaps 25 users worldwide. The app has been selling to one or two new users each year.
For many companies, this wouldn't be much of a success, but considering these users were large organizations, between the price charged and annual support license renewals, it was a good, stable moneymaker.
Despite these plusses, it was actually architected in perhaps the worst way possible. Buried within the application logic was code that would spawn one thread per image (some PDFs can have several hundreds of images!). There were frequent deadlocks, multiple copies of the same objects in memory for no apparent reason (images and PDFs, so very large objects), and three copies of the PDF on the disk (which can easily be 300MB to 800MB).
The reason behind most of these problems? One function that Thomas B. found while on a refactoring expedition:
private static int GetPhysicalFileSize(byte[] Imgbyte, string InputObject, string OutFolder)
{
int PhysicalFileSize = 0;
try
{
ExtractImage(Imgbyte, "Photo_" + InputObject + ".jpg", OutFolder);
var Outfile = OutFolder + "\\Photo_" + InputObject + ".jpg";
if (File.Exists(Outfile))
{
FileInfo fi2 = new FileInfo(Outfile);
PhysicalFileSize = (int)fi2.Length;
}
if (File.Exists(Outfile))
{
File.Delete(Outfile);
}
}
catch (Exception ex)
{
Trace.WriteError(ex);
}
return PhysicalFileSize;
}
So, to determine the size of a byte[] as a file, first, the code would write out the information to disk, read that information and then, finally, delete the file.
All that disk I/O work for the equivalent of this:
return Imgbyte.Length;
Calls to the larger, more cumbersome function were peppered throughout the source, so there was no doubt the handful of customers would be appreciative of the fix, as the benefits would be immediately noticeable.
Thomas B. knew that fixing this bug was a big win, but it was only the tip of the iceberg in the great catalog of apps that he worked on supporting. Thank goodness for steady customers.
About the Author
Mark Bowytz is a contributor to the popular Web site The Daily WTF. He has more than a decade of IT experience and is currently a systems analyst for PPG Industries.