Brendan Enrick

Daily Software Development

Massive Multiplied Methods - Code Audits #5

I am coining a new term and have just googled to make sure that it’s not written anywhere yet. “Massive Multiplied Methods” did not appear in a Google search at all. I establish that I am likely the first person to post those 3 words together in that order on the Internet.

I am sure that everyone reading my blog knows that you should not repeat yourself and that your code should be organized in small cohesive parts.

Sometimes developers don’t do that. Occasionally, we can find places where people completely ignored those two.

The Bad

Imagine a site whose content comes from editors uploading lists of content stored in spreadsheets. Each spreadsheet, for some reason, has to have only 1 type of content (even though the data all has the same fields).

Now imagine that when the system reads each row from the spreadsheet, it needs to map each value onto a specific record from the database. The data is normalized, so what was a string in the excel file needs to link to the correct other table in the database.

The user selects which type of content is being uploaded and then the system will call the method for uploading that content. Yes, it’s just one method. Any other methods called are part of the .NET Framework. Making things worse, the methods are long. Very long. Each one was about 1,500 lines of code.

There were 5 different types. Each of these types has its own 1,500 line method. That’s about 7,500 lines of code for these.

And the best part about those 5 methods? They’re all identical except for 1 string. Literally, the only difference between these methods is the magic string literal used for the type.

The Better

I am not sure what to say here other than. Go learn about parameters in methods and how to create methods to break an algorithm into logical parts. Wow. Just. Wow.

It’s bad enough that the string could have just been in the spreadsheet to begin with, but did it really require 5 separate methods?

To top it off, you could have just accessed the value from the drop down list the user selected from to indicate the type. This was all in a code behind file anyway, so it could have just grabbed that value. (It shouldn’t have been there, but this would at least be slightly better.)

I don’t know how people let copy and paste operations get that bad. To be honest, I am amazed that the methods still matched. I guess people just avoided changing the code because it was too ugly. (That’s a mistake, because we need to clean up the scary code.)

More Code Audit Nuggets

Keep watching for more interesting nuggets of stuff that I’ve seen in codebases.

Loading