Re^8: Corner cases are not always "code smell"

[tilly]

Geez, you've sure drunk the kool-aide, haven't you?

You say that, Reality has corner cases, code doesn't. But in the real world there are cases where people need optimizations. (Less than most people think, but it does happen.) Optimizations naturally lead to having corner cases. Those corner cases should generally not be visible in the interface. Therefore code sometimes does need corner cases that aren't part of reality.

Now you may think that we should never optimize. But just today I wrote about an example where not having the optimization leads to bugs. Underscoring my point even more, that optimization should be invisible in the interface, and the initial implementation of this optimization had bugs in its corner cases. (Interestingly, the corner cases where it had bugs would not have been visible to a code coverage test like Devel::Cover, underscoring even more the need for white-box tests on it!) Therefore if your ideal is fewer bugs, then we have a concrete case where you really want to have corner cases that are not visible in the interface.

In short, you're being blinded by your fanaticism. Real code sometimes does have corner cases that aren't visible in the interface. Those corner cases should either be removed or tested with white-box tests. I agree that often it should be removed, but you go too far when you say that they should always be removed.

[dragonchild]

I'll admit that I've been enjoying some TDD Kool-Aid of late. In part, this is a reaction to my attitude towards testing several years ago and how it's come back to haunt me now. Specifically, with PDF::Template.

In my view of the world (however limited in scope that might be), optimizations are an implementation detail, save where they are required by spec. For example, "This process must happen in 0.1 seconds or less per record." Then, you write tests to verify that not only does this occur for the 1-record situation, but that the process scales linearly as you run 100, 1000, and 1_000_000 records. (Obviously, the latter is an weekend-only test.)

When they are not required by spec, I begin to wonder why optimizations are needed. Assuming that they are, then it's just a different implementation that meets the same specification (as enforced by the unit-tests).

In your case, the original implementation had bugs in it. When you find the bug, it becomes part of the spec (through the testsuite) and you fix the implementation to pass the tests. If that results in a quicker runtime or a reduced memory usage, that's bonus. It's not an optimization - it's correctly functioning code.

Think of this another way - if your code has corner-cases, then your interface also has corner-cases. If the interface doesn't specify to the user that these corner-cases exist, then how will s/he know to take them into account? Or, let's say that there's what you would call a corner-case because of VMS. File::Spec is a good example of this. Well, the spec now includes "This will run on VMS." Whatever is needed to meet the spec is what is needed to be done. And in the File::Spec testsuite, there's a section that deals with VMS, as there should be.

[tilly]

The point that you're ignoring is that to make the spec work, you need to decide internally where and when to switch strategies. (BTW your refusing to call this switch an "optimization" is disingenuous at best.) Saying "this must work" and "that must be fast" says that you need 2 different strategies. It doesn't say where the boundary between them should be. That boundary should be invisible to the user of your code. And therefore you have an internal code boundary that is not part of the external specification.

Furthermore without knowing the exact implementation, you can't specify how it works, because the appropriate boundary depends on the optimization chosen, which is highly implementation-dependent.

[dragonchild]

The point that you're ignoring is that to make the spec work, you need to decide internally where and when to switch strategies.

Yes - any version that implements the spec is correct. If your version doesn't implement the spec, it is incorrect. Any changes you need to make to your version to have it match the spec is internal to that version.

(BTW your refusing to call this switch an "optimization" is disingenuous at best.)

Why? You said that the "optimization" fixed bugs. To me, that's a bugfix. That it optimized the code is often a consequence of correct implementation.

Saying "this must work" and "that must be fast" says that you need 2 different strategies. It doesn't say where the boundary between them should be.

"This must work according to spec." is all I'm saying. The spec contains a speed requirement.

It doesn't say where the boundary between them should be. That boundary should be invisible to the user of your code. And therefore you have an internal code boundary that is not part of the external specification.

Does the code implement the spec? If yes, you may pass Go. If no, then go directly to Jail. If your version implements the spec and is faster than mine, that's wonderful. If it's faster, but it's wrong, then we're not comparing apples.

Furthermore without knowing the exact implementation, you can't specify how it works, because the appropriate boundary depends on the optimization chosen, which is highly implementation-dependent.

This sentence makes absolutely no sense to me. A specification tells you what it does. It most certainly doesn't say how it accomplishes that goal. And, yes, an optimization may certainly be platform-dependent, tools-dependent, etc. If your implementation wants to take those factors into account, it is welcome to do so. However, you're still implementing the spec, which, at the end of the day, is all that matters.