CodeSOD: Recycled Code

Hannes has inherited a legacy project. Like most legacy projects, it has no real documentation, the code is a disorganized mess, and making any change runs a non-zero risk of completely knocking over the house of cards.

What few comments the code has tells us things like this:

//Copied from DateUtil.cspublic static int CW(DateTime Date){ CultureInfo CUI = CultureInfo.CurrentCulture; return CUI.Calendar.GetWeekOfYear(Date, CUI.DateTimeFormat.CalendarWeekRule, CUI.DateTimeFormat.FirstDayOfWeek);}

I suppose it's nice to know that you copied this code from another file, especially as the copied code is both public and static, and thus copying is completely unnecessary. I guess calling DateUtil.CW would have been too confusing, because I mean, what does CW even mean anyway?

Then, he stumbled across a function called StrLength. At a guess, what might you think StrLength does? You might think it's a reinvention of the built-in String.Length function. Well, what if I told you that it returns a string. In fact, take a look at the signature:

private string StrLength(string s, int len, string repl)

What do you think this function does? If you said, "It's a pad left function," then you win" and we have a lot of questions about how your brain works.

private string StrLength(string s, int len, string repl){ string result = string.Empty; result=s; while (result.Length < len){ result = repl + result; } return result;}

So, no, it is not a reinvention of String.Length, but it is a reinvention of String.PadLeft. It also creates loads of unnecessary string instances, and since it does no input checking, could easily receive a multiple character repl value. I guess that's why it's private.

[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!