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

http://bugs.php.net/bug.php?id=50696

by chaostheory • 17 years ago

95 48 17 years ago

bugs.php.net

ddbb• 17 years ago

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..

apexauk• 17 years ago

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..

viraptor2• 17 years ago

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.

joeschmoesky2• 17 years ago

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.

city412• 17 years ago

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.

BrandonM• 17 years ago

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.

nostrademons• 17 years ago

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. ;-)

jrnkntl• 17 years ago

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

Classic.

niyazpk• 17 years ago

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

jrockway2• 17 years ago

"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.

olalonde• 17 years ago

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

archon8102• 17 years ago

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

rbanffy3• 17 years ago

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...

albertsun• 17 years ago

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.

sh1mmer• 17 years ago

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.

wendroid2• 17 years ago

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

viraptor2• 17 years ago

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 ] )"

karam• 17 years ago

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.

alttab• 17 years ago

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.

brandon• 17 years ago

  But simply my $NULL.

Oops!

dugmartin• 17 years ago

This is why I hesitate to release open source software.

jrockway2• 17 years ago

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

vlucas• 17 years ago

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.

smallblacksun• 17 years ago

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

rbanffy3• 17 years ago

+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.

bozmac• 17 years ago

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

jedediah• 17 years ago

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.

bmj• 17 years ago

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.

archon8102• 17 years ago

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

Sumsar• 16 years ago

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?

ars• 17 years ago

I don't see why it should return null.

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

kingkilr• 17 years ago

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

prodigal_erik• 17 years ago

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!

docgnome• 17 years ago

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.

jrgnsd• 17 years ago

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!

mbreese• 17 years ago

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...

Yaa101• 17 years ago

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.

moconnor• 17 years ago

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.

ionfish2• 17 years ago

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

wendroid2• 17 years ago

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

Some might say "PHP developer"

gcb• 17 years ago

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?

InclinedPlane• 17 years ago

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.

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

Related Stories