Ayende @ Rahien

It's a girl

It really happened, legacy programmers tales

Fairy tales always start with “Once upon a time”, and programmers tales starts with “when I was at a client”…

Two days ago I was a client, and the discussion turned to bad code bases, as it often does. One story that I had hard time understanding was the Super If.

Basically, it looked like this:

image

I had a hard time accepting that someone could write an if condition that long. I kept assuming that they meant that the if statements were 50 lines long, but that wasn’t the case.

And then yesterday I had an even more horrifying story. A WCF service making a call to the database always timed out on the first request, but worked afterward. What would be your first suspicion? Mine was that it took time to establish the database connection, and that after the first call the connection resided in the connection pool.

They laughed at my naivety, for it wasn’t connecting to the database that caused the timeout, it was JITting the method that the WCF service ended up calling.

Yep, you got that right, JITting a single method (because the runtime only JIT a single method at a time). I had even harder time believing that, until they explained to me how that method was built:

image

Some interesting stats:

  • It had a Cyclomatic Complexity of either 4,000 or 8,000, the client couldn’t remember.
  • The entire Rhino Mocks codebase fits in 13,000 LOC, so this single method could contain it several times over.

But you know what the really scary part is?

I upgraded from Super If to Black Hole Methods, and I am afraid to see what happen today, because if I get something that top the Black Hole Method, I may have to hand back my keyboard and go raise olives.

Comments

Marcus
08/31/2010 09:06 AM by
Marcus

What is a black hole method?

cbp
08/31/2010 09:16 AM by
cbp

@Marcus I'm guessing a black method is a method that is so massive, even light cannot escape.

Bjorn Coltof
08/31/2010 09:18 AM by
Bjorn Coltof

Toggle all outlining!!! Plz.

tobi
08/31/2010 09:20 AM by
tobi

omg!!1

What was inside of the "find employee with filters" region? Was it the combination of all possible N filters and for each one a hardcoded query?

Koen
08/31/2010 09:23 AM by
Koen

Ayende, you gotta warn your readers before a post like this. I didn't have breakfast yet and I almost had to throw up :o/

Uriel Katz
08/31/2010 09:39 AM by
Uriel Katz

can`t say I saw something that bad,but I saw this kind of things alot of times,is it a company in Israel?

evereq
08/31/2010 11:29 AM by
evereq

Ha, "It had a Cyclomatic Complexity of either 4,000 or 8,000, the client couldn’t remember." :)

How many defects (not even related to JIT optimizations) such method contains than?

And it's only one method... What about whole application??? ;-)

So few things raised in my hand:

a) The client does not even hear about cyclomatic complexity, not just "couldn't remember how much it was" :D

b) I think your fix will have "short life-cycle" and probably you waste time trying to help, because such a code written in .NET simply can't work :) :D :D :D OK, maybe it can, if they will substitute existed JIT compiler with own implementation with black holes support and optimizations :)

But if say frankly, we all from time to time dial with such "legacy" code (or better to say "legacy coding style from native C++) and instead of completely rewrite it, try to "fix" it, fix it and fix it... sometimes it's do work actually ;-)

Paula
08/31/2010 12:55 PM by
Paula

Brillant

Frank Quednau
08/31/2010 01:18 PM by
Frank Quednau

You people may be doing injustice to the programmer. He just commented his code really well.

Alright, it doesn't explain the JIT issue.

tbh, I can't even begin to think how to write such a method.

Steve
08/31/2010 01:27 PM by
Steve

A 75,500+ line method must require a ridiculous level of patience to write assuming that it's not some generated code that is simply cut and pasted in.

Someone with that much patience surely would think at some point "maybe I should break this bad-boy down a bit". I've seen plenty of 1000 line methods in my days, but when you start approaching War and Peace size proportions, even the most stubborn programmer would start re-thinking their design, right?

Juan Gomez
08/31/2010 01:37 PM by
Juan Gomez

75.000 lines of code??? crap, you could fit an entire operating system there... I bet that method makes the server actually come to life and go hunt the "actual" employee.

Panagiotis Kanavos
08/31/2010 02:08 PM by
Panagiotis Kanavos

Reminds me of similar code I encountered in a legacy product created by the Consulting Services branch of a world-known multinational company (for its arrogance as well as its software), including this pearl http://bit.ly/aBsckf

Anyway, you should consider almonds. They have a higher ROI than olives, but you have to pick them in the summer :P

Scott Wojan
08/31/2010 02:17 PM by
Scott Wojan

Ayende,

I believe that qualifies for SUPER massive black hole status. I'd LOVE to hear a high level overview on what possibly could fill that amount of code in a "Get Employee" method.

Frans Bouma
08/31/2010 03:19 PM by
Frans Bouma

whoa :) Someone actually hand-wrote 75K lines in a single method? Amazing :) What's more surprising: the developer apparently has compiled it somehow once, ran it and decided 'it worked'.

Jeff
08/31/2010 05:26 PM by
Jeff

Wow, cyclomatic complexity of 4k-8k? During a code review I found a method with a CC of 1200+ and I thought that was off the charts! That's just sick...btw, Black Hole Method? Best name ever!!!!

Daniel Fros t
08/31/2010 07:02 PM by
Daniel Fros t

black hole methods is a great name. Great post!

Griff
09/01/2010 06:30 AM by
Griff

ok, I work in a department where developers are writing code just like in the example right now!!!

not as extreme, if statements 500+ long, and nested ifs 50+

times.

Unfortunately for me and the other 2 good developers, there is nothing we can do.

Some developers won't listen, and some managers don't care as long at it works...

scooletz
09/01/2010 07:02 AM by
scooletz

@Griff

Have you heard about 'being a change'? If you don't want to hate your job, you either change your job or change your job.

Ashley
09/01/2010 04:02 PM by
Ashley

My jaw literally dropped when I saw the jump from 8 to 52. Care to share a little bit about what we missed in all of those lines?

Graham
09/01/2010 05:33 PM by
Graham

The only person who can debug Black Hole methods is Chuck Norris.....

kioan
09/01/2010 08:55 PM by
kioan

Hahaha! Graham, I agree about Chuck Norris! :P

Tell them that their code needs refactoring. Some critical parts should be written as external utilities in Brainfuck! >:-)

( http://en.wikipedia.org/wiki/Brainfuck)

Mars Saxman
09/01/2010 08:57 PM by
Mars Saxman

There is an entire industry full of terrible programmers who make a living writing custom apps despite having no idea what they are doing.

I used to work on the compiler engine for REALbasic, and we would periodically get bug reports complaining about ludicrously bad compiler performance. At first I had trouble believing them - our largest test projects compiled in maybe 4-5 minutes, tops, so it was hard to imagine how anyone could be waiting 45 minutes to an hour for a compile.

Then we started getting bug reports with actual code attached, and some of the examples we saw were every bit as bad as the examples quoted above. Eventually we learned to stop saying "no real program would ever do this"; every time we would be proven wrong by the ingenious, determined perversity some of our customers used in place of clue.

Seriously: single methods with 40k-50k lines of code. Conditional blocks nested over a hundred levels deep. Tens of thousands of local variables in a method. Single expressions, stacked up over line after line, containing thousands of sub-expressions, all executed as one statement. Classes with thousands of methods and hundreds of thousands of line of source code. Classes with thousands of methods, each containing a single statement. Inheritance hierarchies a hundred classes deep.

tobi
09/01/2010 09:07 PM by
tobi

This is unbelievable. The C# compiler team must be getting incredible bug reports because of the massive adoption of their product.

Arnis L.
09/03/2010 08:57 AM by
Arnis L.

It definitely needs more parameters. Few hundreds of them.

Jason R.
09/07/2010 09:44 PM by
Jason R.

"I upgraded from Super If to Black Hole Methods, and I am afraid to see what happen today, because if I get something that top the Black Hole Method, I may have to hand back my keyboard and go raise olives."

What? I've read this a dozen times, ignoring the grammatical errors, and I'm still not sure what you are trying to say.

Steve
09/08/2010 03:46 PM by
Steve

@scooletz,

That's a rather simplistic view, isn't it? Maybe Griff has a wife and kids so just up and quiting isn't an option, maybe he's in a situation where he has medical problems and if he leaves his current place of employment that he will no longer get medical coverage (very common here in the US), or maybe it's just one aspect of his job he doesn't like, but the rest of it he's okay with so it's a necessary evil.

Or it could be none of the above, but "either change it or leave" rarely is feasible in real life.

I've beaten this horse to death with Ayende via private emails already, but the biggest impediment of "being change" within a Microsoft shop is Microsoft itself. Obviously the mothership isn't saying "build a 76000 line method", but they also don't promote good code quality either (look no further than crap like Oxite or their broken code on MSDN for examples).

This is a complex world, full of complex problems, and "lead or leave" is rarely the right solution.

Jon Galloway
09/13/2010 05:55 AM by
Jon Galloway

I worked at a place that was constantly battling a SQL 6.5 limit on triggers at 8K characters per table (as I remember). Many of their triggers had already been "optimized" to turn each variable to use 1 character names and other minification techniques.

Due to timing issues on the triggers, it was impossible to tell what would happen when a row was inserted into the OrderDetails table, which was a problem because that was the integration point between all of their order systems.

The reason they were stuck on SQL 6.5 compatibility was that they had many Powerbuilder applications that couldn't work with newer versions of SQL Server.

That was the only job I've stayed at for less than a year. I tried to stick it out for a full year, but I just couldn't do it.

Comments have been closed on this topic.