Added intval and ceil to not loose precision during weight conversion

[AlexisPPLIN]

This pull request fixes #27.

Conversion from Kg to Grams now uses intval and ceil.
I've also added this special case to the existing test of ShippingMethod.

Hope this will be merged soon !

[villermen]

Thanks for the PR! This definitely needs to be addressed because we don't want to introduce any float imprecision errors. I've left a comment on the approach. If you're okay with that let me know and I'll also roll this out on the naive pricing casts before merging.

[villermen]

Isn't it better to use round() for this as it corrects float calculation imprecision both ways?:

Suggested change

	(int) $min_weight,
	(int) $max_weight,
	(int)round($data['min_weight'] * 1000.0),
	(int)round($data['max_weight'] * 1000.0),

This also inlines the operation again. If there are no additional steps involved I think introducing a new variable reduces readability.

[villermen]

Suggested change

	$min_weight = intval(ceil($data['min_weight'] * 1000));
	$max_weight = intval(ceil($data['max_weight'] * 1000));

[AlexisPPLIN]

This also inlines the operation again. If there are no additional steps involved I think introducing a new variable reduces readability.

I'm okay with that, see my last commit.

Isn't it better to use round() for this as it corrects float calculation imprecision both ways?:

After a bit of thinking, I think is better to use :

• ceil for min_weight
• floor for max_weight

So we always give a valid range of weights even if round errors occurs.

[villermen]

So we always give a valid range of weights even if round errors occurs.

@AlexisPPLIN We're not trying to fix rounding errors but float math precision errors. We have no say in what way the imprecision will occur, but we always know it will be close to the intended result. I still think rounding inaccuracies away is the better option for this.

Consider in your proposed situation we would receive 1.001 for both the min and max weights:

var_dump(1.001 * 1000); // float(1000.9999999999999)
// Note that this might as well have resulted in 1001.0000000000001 or something close to that.

We would ceil() the min weight and floor() the max weight, leaving us with a range of 1001 to 1000 which is both invalid and weird. We've removed almost 1 for the max and almost 0 for the min because we're biased in how we round.

I hope this managed to convince you =)