toolbar; division by zero on line 56 about autoptimize HOT 15 CLOSED

Fun times: https://3v4l.org/i82FN

from autoptimize.

should be fixed with commit 258bf00

from autoptimize.

This is weird, I don't have this commit applied and when dumping data just to test quickly by editing autoptimizeToolbar.php after L60:

var_dump($stats, $bytes, $class, $size, $percentage); exit;

which results in this output:

array (size=3)
  0 => int 0
  1 => int 0
  2 => int 1464706737
int 0
int 0
string '0.00 B' (length=6)
float 0

And I get no such warning anywhere... The warning means the divisor is 0, not the dividend? Or?
(which would mean $class ended up being 0?)

Or I might be reading things wrong after a full days work...

from autoptimize.

problem is that the divider is based on $class, which is based on log( $bytes , 1024 ). so if $bytes is 0, things indeed might not always go down well :)

from autoptimize.

It's platform dependant, core issue is in use of min() and/or int casting, see: https://3v4l.org/iPdnC

from autoptimize.

Oops! Thanks you @zytzagoo

another way to fix that, maybe could be:
$bytes = $stats[1]|1;

https://3v4l.org/9eguA

from autoptimize.

I'll let you guys agree on the best approach, you're a lot more knowledgeable :-)

from autoptimize.

Don't believe it, maybe all depends on the coffee cups number ;-)

sorry, is better add the condition in this line (for $bytes does not remain with 1):
$class = min( (int) log( $bytes|1 , 1024 ), count( $si_prefix ) - 1 );

BTW: We would also have to change the ceil by floor
$percentage = floor( $bytes / $max_size * 100 );

https://3v4l.org/gRSc3

What do you think?

from autoptimize.

so @zytzagoo, @pablocusto, what is your preferred solution? :-)

from autoptimize.

My fork is using what I've committed above until you guys settle on whatever :)
(at which point I'll change my fork to match upstream if needed [so that I can keep up with future updates more easily])

from autoptimize.

did the tests and @zytzagoo method has better performance.

this is the test: https://3v4l.org/ZJ0ss

but I think we should change the ceilby floor to round the percentage value.

from autoptimize.

Great, @zytzagoo could you issue a PR with just this specific fix?

from autoptimize.

Didn't touch ceil since it's an unrelated change really (should be a separate commit/pr/issue basically).

Not really sure which is better -- my gut says to stay with ceil since it "errors up" and this is about percentages shown to the users (meaning if they're close to full cache, it's better to be notified "sooner" rather than "later" -- but it's nitpicking, I mean, we're talking 1% up/down difference in the worst case, no?)

Plus, ceil doesn't require any further code changes and I'm lazy like that :)

from autoptimize.

thanks for the PR, merged! :)

I prefer to stick with ceil, as it's weird to have 0% when you have e.g. 10 files 312KB.

from autoptimize.

You're right, I had not thought that.
That just to 5 Mb are 1%.

from autoptimize.