Improved logarithm of base 2 and 10 of standard math functions #658
Conversation
| if (ZEND_NUM_ARGS() == 1) { | ||
| RETURN_DOUBLE(log(num)); | ||
| } | ||
| if (base <= 0.0) { |
There was a problem hiding this comment.
Why did you move this statement below most of the others? Exit early seems like a better idea to me.
There was a problem hiding this comment.
because of it's an error condition that shouldn't be the normal case. So on calculate one of the pre-defined base logarithms the one operation to check against <= 0 isn't needed.
There was a problem hiding this comment.
Maybe I'm misunderstanding it, but it looks like for base 2, it skips the error check entirely? :/
There was a problem hiding this comment.
@TazeTSchnitzel for base 2 it will either get caught by log2() or falls down into log(num) / log(2.0).
There was a problem hiding this comment.
log2 is a C function, yes? It's not going to call RETURN_FALSE.
There was a problem hiding this comment.
@TazeTSchnitzel I'm afraid you've lost me :) why should anything error out when base is 2?
There was a problem hiding this comment.
...oh, right. I'm an idiot, ignore me.
By the way, would this be nicer if structured as a switch statement?
|
Comment on behalf of pollita at php.net: Merged |
... to reduce rounding errors.
Internally on
log(x, 2)the nativeCfunctionlog2(x)and onlog(x, 10)the nativeCfunctionlog10(x)will be called.Because of after this PR the following will be the same in PHP
log10(x)===log10(x)the functionlog10is needless and could be deprecated/removed. I will create an RFC if this PR was merged.This PR is against PHP-5.4 as there is no BC break.
Please merge into PHP-5.4 up to master.
Please tell me if I did something wrong.