Think about Winnie the Pooh.
Winnie is the sweetest, kindest, lovable ol' Pooh Bear you ever did see.
Winnie the Pooh makes me happy.
You know what doesn't make me happy?
Code that looks like this.
That code looks pretty complex, to new eyes. Especially if you're looking at it after trying to figure out why someone else's library, which you've cloned, doesn't pass all of its regression tests. You might look at that and have a few moments of doubt. Concern. Do I understand how this library is used?
Then you stare at it for a minute. You realize that the intent of this Miracle of Indirectness is to find if a key is contained in a collection of registered services. That's not hard.
One can easily think of doing that in a way that makes sense. You basically have a Map between Service and Configuration. If your test is supposed to modify,or not modify, one of these values, you can easily ask.
Instead, this test prefers to go through the list once, in place, instead of building the map. Ah. Efficiency. How delightful. Except (pun-ish-ment) that the loop uses JUnit's assertEquals() in the body of the
try block, which throws a ComparisonFailure when the condition is not true.This means that we're using Exceptions for flow control. DON'T USE EXCEPTIONS FOR FLOW CONTROL!!!
They expect exceptions to be set up often but thrown rarely, and they usually let the throw code be quite inefficient. Throwing exceptions is one of the most expensive operations in Java, surpassing even new. On the other hand, don't forget the first rule of optimization (RulesOfOptimization).So, in trying to be efficient, we've completely destroyed the point of efficiency? Well, at least the bottom of the method is self-documenting.
I know, I know. I shouldn't care so much about what I do. "It's all bullshit anyway", right? If it works, yay! If not, "make it work, whatever it takes." It's not in fashion to try to be, ya know...Elegant. Precise. Crisp.
Whatever. It's not my library. I don't have to care. I just have to try to make a contribution that will fix my problem, and if it inadvertently breaks half the world, e.g. hundreds of other packages that depend on this one....FUCK EM! Right?
Write tests for your new code. Don't worry about the horrible legacy you're trodding upon...But then I have to look at code like this, and just ignore it.
WTF bro? That's not even a test. If you're going to test for the persistence of a dynamically generated file in a transient environment, that's hard. But probably doable. It involves actually writing a condition to test.
But writing tests like this is worse than just leaving fail(). This test, naively, just by looking at a report, looks like it's doing something intelligent. Then you look at it. You know what this causes?
He's coming for your face, honey. He's.coming.for.your.face.
