Saturday, 22 February 2014

ColdFusion: Fixing any bug has backwards compatibility concerns

G'day:
I'm going to bang on about isValid() and integers some more. You've been warned.

Right, so I was horrified to see that this is the case in ColdFusion:

isValid("integer", "$,1,2,$,2352345,$"): YES

And, what's more, having used ColdFusion to establish that $,1,2,$,2352345,$ is in fact an integer, if I then try to use it as an integer, ColdFusion breaks. At least it gets this half right. I discuss this at undue length in "Can we please agree that Adobe is not the arbitor of what constitutes an integer?".

Good old Rupesh poked his head above the parapet briefly, and offered a better response than one might expect here:

Avatar




There is no doubt that this behavior is incorrect. It is obviously wrong and it should be corrected. However, it has been like this forever and making such a fundamental change has a great potential to break a lot of applications. We dont want to do that in this release. As Rakshith has already communicated, we plan to take up such changes in 'Dazzle' where we will correct the behavior without worrying about backward compatibility.

This is an improvement over his earlier comment on the topic:

  • Rupesh Kumar
    2:53:06 AM GMT+00:00 Apr 24, 2012
    This has always been the behavior and changing this would result in backward compatibility issue. It will not be fixed.

There's a fundamental error in Rupesh's internal logic here. To suggest that fixing a bug somehow has backwards compatibility issues is utterly specious.

Intrinsically "a bug" causes some manner of behaviour to occur. It's a bug because the behaviour is incorrect. And, intrinsically, if a bug is fixed, it is implicit that the previous behaviour will change. That's the entire reason for fixing the bug: to replace incorrect behaviour with correct behaviour. So, on the face of it (and this is why I say his position is specious, not simply out and out frickin' stupid... although it is also that), this creates a backwards compatibility consideration. Because the new (correct) behaviour will be different from old (incorrect) behaviour.

But this is the nature of fixing bugs. The behaviour of the buggy functionality changes because it becomes "not buggy". If we were to clamour "backwards compatibility!" each time we considered fixing a bug... no bugs could ever possibly be fixed. And clearly we do fix bugs (obviously). So "backwards compatibility" is not grounds for not fixing bugs. Because it is not actually a valid consideration.



OK. I'm being slightly obtuse. There could be a situation in which a bug is such that some functionality "almost works"; it does - say - 95% of the job, and we still use that function and do the last 5% ourselves. xmlFormat() is a good example: it escapes most meaningful characters in XML, but misses a few. So one can use xmlFormat() to do most of them, and then just fix the last few if needs must. And perhaps this is such a ubiquitous function that everyone knows it only does 95% of the job, and we all do the last 5% ourselves. If xmlFormat() was subsequently fixed, it might actually break some people's "last 5% fix". So they will have a backwards compatibility issue. This is fine. The problem is not that they now have to fix their code, it's that the had to fix it in the first place! The problem is the bug, not any rework that needs to be done to remove an in-house fix for the bug.

So even in this situation, it's still illegitimate to claim "backwards compatibility" as grounds for not fixing a bug.

However in the case of isValid("integer"), there is not even this consideration. As Rupesh now seems to agree: it's not up to him to decide what is and is not an integer. This is prescribed in mathematics.

There will definitely be code out there that works around isValid()'s failings (and the failings extend well beyond its integer validation: it really is a shockingly badly implemented piece of functionality); the code will be possibly using isValid() to remove the most obvious falsehoods (eg: isValid("integer", "purple"), and then go on to also validate for non-integers that isValid() lets through, like "1,2,3,4" (also, according to isValid(), an integer!). However in this case, if isValid() was fixed, all that would happen is there'd be some invalid code left in the code base. Not a problem.

Rupesh would have a case if what I was suggest is that isValid("integer") would accept a long integer as a value, but in reality for some reason it should only be a vanilla integer. And if we made this change, then a lot of people checking for values greater than the scope of an integer would suddenly be getting validation failures. Or perhaps we were quibbling over whether "123.00" is an integer (I can see an argument that it should  be; in a loosely-typed language, there is a sensible casting of that to "123" for integer operations). But we're not quibbling over that, as "123.00" already doesn't qualify as an integer as far as CF is concerned.

What I think Rupesh is doing is concocting another almost certainly fictitious situation in which code might be out there that actually relies on isValid("integer", "1,2,3,4") being true. And if suddenly that started reporting false, then code would break. I would like to say two things about that:
  • Rupesh: prove it. Produce some real-world, in the wild code that is like this. This is such a far-fetched scenario that it falls under the realm of "extraordinary claims require extraordinary evidence". But you can't just produce a single instance. You need to provide sufficient evidence that this problem is sufficiently epidemic that it is a real justification for not fixing this issue.
  • But... so - the hell - what? If people are so stupid as to leverage isValid() to validate something that clearly isn't an integer but their code relies on isValid() returning true... they do not deserve to hold the rest of the ColdFusion community to ransom because they're stupid. Let their code break. It's not Adobe's job to protect bad code from itself. Stop trying to sink to the lowest common denominator. This sort of behaviour really hurts ColdFusion's perception in the industry. It can't even work out what an integer is! And nor - apparently - can its developers! How embarrassing is that?
But even then... it's not like ColdFusion has this endemic inability to determine what an integer is, and will accept values like "$1,234" as an integer. This value will error if it's used as a numeric (not even integer, but numeric) anywhere else in CFML except for the very function which is designed to say "no, that's not an integer". So I challenge you further, Rupesh... what's the scenario under which one might "legitimately" want isValid() to pass "$1,234" as an integer, but then will just error if it's then used as an integer. Go on... tell us. Again: no works of fiction please; actual real world situations.

Now... given ColdFusion's flexibility with data-typing, I'd be prepared for some non-integer values to pass a "non-strict" integer test:
  • $100
  • $100.00
  • $1,000.00
  • €1.000,00 (provided the locale is appropriate for the given numeric syntax)
  • ($100) (alternate monetary syntax for a negative value)
These are human-world situations in which I could conceivably agree to allow those as integer values (but, really, only provided they could then be used as integer values in other operations). But I would definitely then want to have a strict mode in which only accept actual mathematical- / comp-sci-valid integers.

That said, however the things is fixed: do make sure you document the behavioural changes in the release notes, eg: "isValid() used to incorrectly pass 1,2,3,4 as an integer, and it now doesn't, so be mindful of this if your code leverages this incorrect behaviour". Not that anyone being so daft as to do this will read release notes, but at least you've done what you can to protect them from themselves (and your own previous folly).


Rupesh does concede that this can be looked at in ColdFusion 12 (another two years away!) because they are committed to breaking backwards compatibility in CF12. Ballocks to that. There is no real-world backwards compat issue here, so you don't need to wait until CF12.

Just fix it. And I don't just mean this issue: I mean all issues where you're speciously contriving "backwards compatibility" as an excuse to not fix things.

Stop being embarrassing, and stop trying to perpetuate ColdFusion's mediocrity.

--
Adam