PHP Bug: #50696: number_format when passed a 0, returns null | Hacker News Reader

PHP Bug: #50696: number_format when passed a 0, returns null

bugs.php.net

95 pointschaostheory48 comments

Interesting discussion in there, but I think the PHP guys are right on this one. You should never rely on undefined behavior of any API for mission critical code.

Always use what is documented so you don't have to cry later..

When I first read it quickly I thought it sounded a fair complaint, then I saw this comment and figured I should think again. On 2nd read I picked up the subtleties re: undocumented behaviour, reason behind the change & effect reverting it would have and yes, I think I agree after all.

Different 'bug', similar theme: I'd be v. interested to hear what HN folks think about http://bugs.php.net/bug.php?id=47494. I explained the problem here: http://insomanic.me.uk/post/191397106/php-htmlspecialchars-h...

Summary: PHP 5.3 introduces a scenario where:

  display_errors=off, log_errors=on => warning msg is logged (but not displayed, of course..)   
  display_errors=on => NO warning is logged OR displayed(?!)

Took me ages to figure out, that one did..

I'd take the other side... but I can understand them choosing either way.

My vote for the other side is because php guys didn't document the change. Other pages show exactly what happens in strange cases - like in "If delimiter is an empty string (""), explode() will return FALSE." There's no mention about the change on http://php.net/manual/en/function.number-format.php and they list the result as "string" without any other notes. Making the function backwards compatible wouldn't hurt anyone either, because it would restore the behaviour for people using "" and would change nothing for ones using normal values all the time and ones who started casting to a number because of this change. But yes - it's pretty much php guys' call.

Seems reasonable. And I don't think you guys got the humor when he said "Escalate". You all want to think you're SO smart and nobody else can pick his nose. Obviously he knew who he was talking to, do you really think he uses PHP and doesn't know who he was dealing with?

Anyway, saying they wouldn't fix it is fine, but the bitchy answers he got to legitimate questions just shows how juvenile R is.

The PHP guys are right in the same sense a store is right that it doesn't have to take a return without a receipt. But most stores will do so anyway in order to maintain good will. Whenever we come across undocumented or undefined behavior that will change in a new release, we try very hard to not change it unless it's absolutely necessary.

I would argue that it makes a lot more sense for a non-number to return NULL than to return 0. If it returns 0, how do you distinguish between "0" and ""? At least with a NULL return value you can use the function to see if you're even looking at a number at all.

Sure. And I haven't used PHP in about a decade. But it sounds to me like this was overlooked by them and so returned 0 for many years. Despite being undocumented behavior, fixing it causes a breaking change. On my team whenever we come into this situation and realize that fixing overlooked things can actually "break" people, we really think twice about the fix. Even if in theory the fix on its own is totally the right thing to do.

> fixing it causes a breaking change. On my team whenever we come into this situation and realize that fixing overlooked things can actually "break" people, we really think twice about the fix.

Answered in the 2nd comment from Rasmus:

If this was changed in a minor version, I'd agree with you on the BC change, but we have been working on catching these weird edge-case scenarios that lead to unexpected bugs.

i.e. they thought about it and decided this version was an acceptable one for making breaking changes.

This would work if programmers actually documented their code.

I remember posting similar sentiments on Reddit about 3 years ago and getting downmodded to oblivion. In hindsight, I think I was wrong and the herd was right. If you restrict yourself to what's documented, you'll miss out on most of the interesting and cutting-edge stuff. And that's what lets you build a cool, differentiated, useful project.

Instead, I think you should budget time and money for crying later. ;-)

"Escalate? Oh how I wish I had someone to escalate to."

Classic.

I like his humility. Many people in these situations would be like: "Do you know who I am?"

"Do you know who I am?" is probably a better answer. The "humble" answer is just an amusing in-joke at the expense of the poster. ("Haha, look at that dumb n00b who doesn't know who Rasmus is!" There is even a comment below that pretty much says this exactly.)

"I'm the creator of PHP. I am not going to fix this," would have ended the discussion much sooner and with less hurt feelings.

Does this guy realize he's asking the founder of PHP (rasmus@php.net) to "escalate this to someone who can answer". lol

Clearly, he doesn't, until the last comment.

You know what's more depressing?

This clueless guy is one of the (wild guess) 5% of PHP users who can file a bug report and follow it...

I would guess even lower. I'm not sure why that depresses you though. In my mind, that's actually a sign of success for the language.

I wouldn't guess. why guess? why not just not quote a number at all since you have no idea?

I'm going to miss Rasmus at Yahoo!. I've spent a fair amount of time with him on the road giving lectures to college students and at conferences.

This is one of the reasons he's such a great leader. He's got a sense of humour, but he still explains it to you no matter what. Good guy.

he sure does wish that :

"I'm not a real programmer. I throw together things until it works then I move on. "

http://en.wikiquote.org/wiki/Rasmus_Lerdorf

I'm definitely on the PHP guys with this. A big criticism against PHP is that it's inconsistent in numerous ways. This bugfix is obviously part of an effort to standardize function behaviour.

Go PHP!

But the problem with this is that those inconstancies have been around for so long that people have come to rely on them. So when you try to "fix the glitch", people get upset.

So, when you say it's part of an effort to standardize behavior, I think you're right. But is this the right way to go? I mean, was the old behavior that bad? Why not just flag it as a warning and mark it as deprecated functionality? Then wait until version 6 to actually force the change.

I don't have a horse in this race, since I stopped using PHP when 5 was just coming out. One of those reasons was to avoid stuff like this...

I do not agree, if the programmer relies on idiosyncrasies of a system then it's the programmer's fault, no matter how long these idiosyncrasies were in the system. It's the programmers job to anticipate on that. It is also a fallacy to think (especially for a programmer) that your program is done when finished, last 40 years have shown that a programmer always has to do maintenance just like buildings and bridges and other infra.

Further I agree with what Rasmus states as one of his last remarks in that bug thread: "Wow, a classic case of how not to treat unpaid volunteers who provide critical pieces of your money-making infrastructure."

In other words, go make yourself a programming language if you have a big mouth towards volunteers when you scheme of making money with their system fails you, the bloody arrogance.

This is funny because it's not really a bug at all. I side with the PHP guys on this one. Passing in a string to a number_format function is not even proper form in the first place. I do agree that it should always default to 0 instead of null because it is a numeric function, but still. The usage is the main problem in this case - and who wants to fix a bug for an asshole anyways?

A lesson to all who rely on edge cases and un-documented behaviors for functionality.

number_format returns a string - I would expect it to return null rather than 0 for bad input

+1.

It should thrown an exception and abort the script with an ugly error traceback.

If the documentation says "float" and you can hand it a numerical string, then either the function or its documentation have a bug that needs correcting.

I wouldn't go as far as throwing an exception. A notice maybe.

Asking a function that formats a number and that is defined that way in the documentation to format a string is a programming mistake. As the discussion clearly states, it's a minor change in application code that would make it both clearer and more robust.

I would go as far as recommend using a USB shock device to issue a warning to the programmer every times a mistake like this is made. ;-)

A bit unrelated to the discussion... but will anyone be surprised if they get a couple of cents more (or less) on their retirement?

From the report: "Each of those changes will have to be coded, tested, written-off, released, tested by the clients since this is tax data and has to be precise for tax planning and retirement planning."

From the documentation: "string number_format ( float $number [, int $decimals ] )"

A couple of cents off individually might not matter. A couple of cents off cumulatively across a few hundred thousand individuals over a few years does matter.

From the documentation is correct!

Passing a string instead of a float and expecting it to behave a certain way is undocumented. Oh my! Relying on undocumented behavior... a simple duh in the production world.

I'm not adding to the conversation, and I realize this. But simply my $0.02.

  But simply my $NULL.

Oops!

Although Rasmus is quite clearly right to have fixed the undefined behaviour and is probably quite charming in person, if anyone in our company responded to one of our customers like that, well, there'd be trouble.

He could've saved himself a lot of grief if his first reply had been:

Hi,

I'm sorry to hear this change has broken your existing code. We've been cleaning up undefined behaviours such as the one you're relying on in this release and this particular fix has been reviewed and accepted by the community over a 3 month period, so there's no chance to revert it now.

Going forward, you can either patch your code to stop relying on the undocumented behaviour (e.g. cast the string to a float) or you're also free to modify the PHP source to return to the previous behaviour - one of the benefits of relying on an open source framework.

Best regards, Rasmus Lerdorf, creator of PHP

Sadly, this would never have appeared on HN and thus brightend up my Monday morning.

Presumably the people in your company aren't unpaid volunteers.

> From September 2002 to November 6th 2009, he had been employed by Yahoo! Inc. as an Infrastructure Architecture Engineer.

Some might say "PHP developer"

They might indeed, but the person filing the bug report in question (in their capacity as a user of PHP, at any rate) is not a customer of Yahoo! Inc., and thus Rasmus' employment there is irrelevant to the point at hand.

That point, rhetoric aside, is that the reporter is not party to a commercial contract with the people who fix bugs in PHP, whoever they might happen to be. Consequently, there should not be the "trouble" that moconnor asserted would occur should anyone in his organisation respond to a customer in that manner. The two parties simply do not have that relationship.

The phrase "unpaid volunteer" comes from Rasmus himself, in the bug report under discussion: "Wow, a classic case of how not to treat unpaid volunteers who provide critical pieces of your money-making infrastructure."

he could also tested his damn application before upgrading php.

in his make up world that makes it too dificult to fix his code, he would have to go trhu 7 levels of testing hell... so why not apply it to the platform?

Interesting how the reason that this bug is affecting the original poster is that a new version of the language is being used. The code can't be changed because "We have number_format in literally thousands of places across 50 or 60 separate products. Each of those changes will have to be coded, tested, written-off, released, tested by the clients since this is tax data and has to be precise for tax planning and retirement planning."

It seems to me that the entire process of testing and writing off is just as important when changing the target platform as it is when changing some of the API calls.

Agreed. When we update our platform, the entire product goes through regression tests (not necessarily a full re-run of every test, though). If we change any code to accommodate the new version, those modules are completely re-tested.

If they write tax software and don't do any formal testing, I'd seriously hesitate to use their product.

I don't see why it should return null.

If "" == 0, meaning "" is coerced to 0, shouldn't it coerce to 0 here too?

Equality isn't transitive in PHP :/. Of course, IMO, the real coding horror here is that in an error circumstance no exception is raised.

This. There are dozens of situations where, given instructions that simply don't make sense, PHP arbitrarily picks some half-baked behavior instead of giving an error. So much so that I think the original poster is out of line for even considering tax planning in a language that wants to guess what you might have wanted to happen!

While I understand (and agree with) the change to try and standardize PHP behavior, I think that in this case the parent comment is the crux of the issue. Seems like returning NULL violates the principle of least surprise to me.

One wonders, how would it be possible for this gentleman to work under the onus of such a horrifically restrictive enterprise-y change control board and yet have the ability to upgrade the toolset for 50 or 60 seperate products, his words, without going through said onerous change control process? It seems like there is a giant gaping hole in this company's processes, not surprising as it sounds like a horrible place for anyone with talent to work.

This is why I hesitate to release open source software.

Because you're afraid to tell a troll on the Internet "no"?

The ending (the last comment). It is epic.

I ran into this same problem. Reread the original bug report, ignoring the rest. The poster respectfully made a simple query as to expected behavior. From rasmus first response the poster was treated flippantly and with disregard. And the poster didn't wilt, but instead answered toe to toe. Anyway, now I have this code where user's skip answering fields (damn users!) and my code used to fill in the zeroes (or commas in the thousands). But now my code no worky. Will that cast to float work? I was doing this elaborate if($in==""){0:number_format($in);} or whatever, but do I only have to cast to float?

Reply on news.ycombinator.com