Skip to content

Commit

Permalink
feature #35729 [Form] Correctly round model with PercentType and add …
Browse files Browse the repository at this point in the history
…a rounding_mode option (VincentLanglet)

This PR was squashed before being merged into the 5.1-dev branch.

Discussion
----------

[Form] Correctly round model with PercentType and add a rounding_mode option

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix #35296
| License       | MIT
| Doc PR        | symfony/symfony-docs#13138

Commits
-------

d97565d [Form] Correctly round model with PercentType and add a rounding_mode option
  • Loading branch information
fabpot committed Mar 16, 2020
2 parents e0bddee + d97565d commit 06a1a1b
Show file tree
Hide file tree
Showing 4 changed files with 229 additions and 4 deletions.
1 change: 1 addition & 0 deletions src/Symfony/Component/Form/CHANGELOG.md
Expand Up @@ -14,6 +14,7 @@ CHANGELOG
is deprecated. The method will be added to the interface in 6.0.
* Implementing the `FormConfigBuilderInterface` without implementing the `setIsEmptyCallback()` method
is deprecated. The method will be added to the interface in 6.0.
* Added a `rounding_mode` option for the PercentType and correctly round the value when submitted

5.0.0
-----
Expand Down
Expand Up @@ -23,6 +23,55 @@
*/
class PercentToLocalizedStringTransformer implements DataTransformerInterface
{
/**
* Rounds a number towards positive infinity.
*
* Rounds 1.4 to 2 and -1.4 to -1.
*/
const ROUND_CEILING = \NumberFormatter::ROUND_CEILING;

/**
* Rounds a number towards negative infinity.
*
* Rounds 1.4 to 1 and -1.4 to -2.
*/
const ROUND_FLOOR = \NumberFormatter::ROUND_FLOOR;

/**
* Rounds a number away from zero.
*
* Rounds 1.4 to 2 and -1.4 to -2.
*/
const ROUND_UP = \NumberFormatter::ROUND_UP;

/**
* Rounds a number towards zero.
*
* Rounds 1.4 to 1 and -1.4 to -1.
*/
const ROUND_DOWN = \NumberFormatter::ROUND_DOWN;

/**
* Rounds to the nearest number and halves to the next even number.
*
* Rounds 2.5, 1.6 and 1.5 to 2 and 1.4 to 1.
*/
const ROUND_HALF_EVEN = \NumberFormatter::ROUND_HALFEVEN;

/**
* Rounds to the nearest number and halves away from zero.
*
* Rounds 2.5 to 3, 1.6 and 1.5 to 2 and 1.4 to 1.
*/
const ROUND_HALF_UP = \NumberFormatter::ROUND_HALFUP;

/**
* Rounds to the nearest number and halves towards zero.
*
* Rounds 2.5 and 1.6 to 2, 1.5 and 1.4 to 1.
*/
const ROUND_HALF_DOWN = \NumberFormatter::ROUND_HALFDOWN;

const FRACTIONAL = 'fractional';
const INTEGER = 'integer';

Expand All @@ -31,6 +80,8 @@ class PercentToLocalizedStringTransformer implements DataTransformerInterface
self::INTEGER,
];

protected $roundingMode;

private $type;
private $scale;

Expand All @@ -42,7 +93,7 @@ class PercentToLocalizedStringTransformer implements DataTransformerInterface
*
* @throws UnexpectedTypeException if the given value of type is unknown
*/
public function __construct(int $scale = null, string $type = null)
public function __construct(int $scale = null, string $type = null, ?int $roundingMode = self::ROUND_HALF_UP)
{
if (null === $scale) {
$scale = 0;
Expand All @@ -52,12 +103,17 @@ public function __construct(int $scale = null, string $type = null)
$type = self::FRACTIONAL;
}

if (null === $roundingMode) {
$roundingMode = self::ROUND_HALF_UP;
}

if (!\in_array($type, self::$types, true)) {
throw new UnexpectedTypeException($type, implode('", "', self::$types));
}

$this->type = $type;
$this->scale = $scale;
$this->roundingMode = $roundingMode;
}

/**
Expand Down Expand Up @@ -166,7 +222,7 @@ public function reverseTransform($value)
}
}

return $result;
return $this->round($result);
}

/**
Expand All @@ -179,7 +235,58 @@ protected function getNumberFormatter()
$formatter = new \NumberFormatter(\Locale::getDefault(), \NumberFormatter::DECIMAL);

$formatter->setAttribute(\NumberFormatter::FRACTION_DIGITS, $this->scale);
$formatter->setAttribute(\NumberFormatter::ROUNDING_MODE, $this->roundingMode);

return $formatter;
}

/**
* Rounds a number according to the configured scale and rounding mode.
*
* @param int|float $number A number
*
* @return int|float The rounded number
*/
private function round($number)
{
if (null !== $this->scale && null !== $this->roundingMode) {
// shift number to maintain the correct scale during rounding
$roundingCoef = pow(10, $this->scale);

if (self::FRACTIONAL == $this->type) {
$roundingCoef *= 100;
}

// string representation to avoid rounding errors, similar to bcmul()
$number = (string) ($number * $roundingCoef);

switch ($this->roundingMode) {
case self::ROUND_CEILING:
$number = ceil($number);
break;
case self::ROUND_FLOOR:
$number = floor($number);
break;
case self::ROUND_UP:
$number = $number > 0 ? ceil($number) : floor($number);
break;
case self::ROUND_DOWN:
$number = $number > 0 ? floor($number) : ceil($number);
break;
case self::ROUND_HALF_EVEN:
$number = round($number, 0, PHP_ROUND_HALF_EVEN);
break;
case self::ROUND_HALF_UP:
$number = round($number, 0, PHP_ROUND_HALF_UP);
break;
case self::ROUND_HALF_DOWN:
$number = round($number, 0, PHP_ROUND_HALF_DOWN);
break;
}

$number = 1 === $roundingCoef ? (int) $number : $number / $roundingCoef;
}

return $number;
}
}
18 changes: 16 additions & 2 deletions src/Symfony/Component/Form/Extension/Core/Type/PercentType.php
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\Form\Extension\Core\Type;

use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\Extension\Core\DataTransformer\NumberToLocalizedStringTransformer;
use Symfony\Component\Form\Extension\Core\DataTransformer\PercentToLocalizedStringTransformer;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\Form\FormInterface;
Expand All @@ -25,7 +26,11 @@ class PercentType extends AbstractType
*/
public function buildForm(FormBuilderInterface $builder, array $options)
{
$builder->addViewTransformer(new PercentToLocalizedStringTransformer($options['scale'], $options['type']));
$builder->addViewTransformer(new PercentToLocalizedStringTransformer(
$options['scale'],
$options['type'],
$options['rounding_mode']
));
}

/**
Expand All @@ -43,6 +48,7 @@ public function configureOptions(OptionsResolver $resolver)
{
$resolver->setDefaults([
'scale' => 0,
'rounding_mode' => NumberToLocalizedStringTransformer::ROUND_HALF_UP,
'symbol' => '%',
'type' => 'fractional',
'compound' => false,
Expand All @@ -52,7 +58,15 @@ public function configureOptions(OptionsResolver $resolver)
'fractional',
'integer',
]);

$resolver->setAllowedValues('rounding_mode', [
NumberToLocalizedStringTransformer::ROUND_FLOOR,
NumberToLocalizedStringTransformer::ROUND_DOWN,
NumberToLocalizedStringTransformer::ROUND_HALF_DOWN,
NumberToLocalizedStringTransformer::ROUND_HALF_EVEN,
NumberToLocalizedStringTransformer::ROUND_HALF_UP,
NumberToLocalizedStringTransformer::ROUND_UP,
NumberToLocalizedStringTransformer::ROUND_CEILING,
]);
$resolver->setAllowedTypes('scale', 'int');
$resolver->setAllowedTypes('symbol', ['bool', 'string']);
}
Expand Down
Expand Up @@ -79,6 +79,109 @@ public function testReverseTransform()
$this->assertEquals(2, $transformer->reverseTransform('200'));
}

public function reverseTransformWithRoundingProvider()
{
return [
// towards positive infinity (1.6 -> 2, -1.6 -> -1)
[PercentToLocalizedStringTransformer::INTEGER, 0, '34.5', 35, PercentToLocalizedStringTransformer::ROUND_CEILING],
[PercentToLocalizedStringTransformer::INTEGER, 0, '34.4', 35, PercentToLocalizedStringTransformer::ROUND_CEILING],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.45', 3.5, PercentToLocalizedStringTransformer::ROUND_CEILING],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.44', 3.5, PercentToLocalizedStringTransformer::ROUND_CEILING],
[null, 0, '34.5', 0.35, PercentToLocalizedStringTransformer::ROUND_CEILING],
[null, 0, '34.4', 0.35, PercentToLocalizedStringTransformer::ROUND_CEILING],
[null, 1, '3.45', 0.035, PercentToLocalizedStringTransformer::ROUND_CEILING],
[null, 1, '3.44', 0.035, PercentToLocalizedStringTransformer::ROUND_CEILING],
// towards negative infinity (1.6 -> 1, -1.6 -> -2)
[PercentToLocalizedStringTransformer::INTEGER, 0, '34.5', 34, PercentToLocalizedStringTransformer::ROUND_FLOOR],
[PercentToLocalizedStringTransformer::INTEGER, 0, '34.4', 34, PercentToLocalizedStringTransformer::ROUND_FLOOR],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.45', 3.4, PercentToLocalizedStringTransformer::ROUND_FLOOR],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.44', 3.4, PercentToLocalizedStringTransformer::ROUND_FLOOR],
[null, 0, '34.5', 0.34, PercentToLocalizedStringTransformer::ROUND_FLOOR],
[null, 0, '34.4', 0.34, PercentToLocalizedStringTransformer::ROUND_FLOOR],
[null, 1, '3.45', 0.034, PercentToLocalizedStringTransformer::ROUND_FLOOR],
[null, 1, '3.44', 0.034, PercentToLocalizedStringTransformer::ROUND_FLOOR],
// away from zero (1.6 -> 2, -1.6 -> 2)
[PercentToLocalizedStringTransformer::INTEGER, 0, '34.5', 35, PercentToLocalizedStringTransformer::ROUND_UP],
[PercentToLocalizedStringTransformer::INTEGER, 0, '34.4', 35, PercentToLocalizedStringTransformer::ROUND_UP],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.45', 3.5, PercentToLocalizedStringTransformer::ROUND_UP],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.44', 3.5, PercentToLocalizedStringTransformer::ROUND_UP],
[null, 0, '34.5', 0.35, PercentToLocalizedStringTransformer::ROUND_UP],
[null, 0, '34.4', 0.35, PercentToLocalizedStringTransformer::ROUND_UP],
[null, 1, '3.45', 0.035, PercentToLocalizedStringTransformer::ROUND_UP],
[null, 1, '3.44', 0.035, PercentToLocalizedStringTransformer::ROUND_UP],
// towards zero (1.6 -> 1, -1.6 -> -1)
[PercentToLocalizedStringTransformer::INTEGER, 0, '34.5', 34, PercentToLocalizedStringTransformer::ROUND_DOWN],
[PercentToLocalizedStringTransformer::INTEGER, 0, '34.4', 34, PercentToLocalizedStringTransformer::ROUND_DOWN],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.45', 3.4, PercentToLocalizedStringTransformer::ROUND_DOWN],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.44', 3.4, PercentToLocalizedStringTransformer::ROUND_DOWN],
[PercentToLocalizedStringTransformer::INTEGER, 2, '37.37', 37.37, PercentToLocalizedStringTransformer::ROUND_DOWN],
[PercentToLocalizedStringTransformer::INTEGER, 2, '2.01', 2.01, PercentToLocalizedStringTransformer::ROUND_DOWN],
[null, 0, '34.5', 0.34, PercentToLocalizedStringTransformer::ROUND_DOWN],
[null, 0, '34.4', 0.34, PercentToLocalizedStringTransformer::ROUND_DOWN],
[null, 1, '3.45', 0.034, PercentToLocalizedStringTransformer::ROUND_DOWN],
[null, 1, '3.44', 0.034, PercentToLocalizedStringTransformer::ROUND_DOWN],
[null, 2, '37.37', 0.3737, PercentToLocalizedStringTransformer::ROUND_DOWN],
[null, 2, '2.01', 0.0201, PercentToLocalizedStringTransformer::ROUND_DOWN],
// round halves (.5) to the next even number
[PercentToLocalizedStringTransformer::INTEGER, 0, '34.6', 35, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[PercentToLocalizedStringTransformer::INTEGER, 0, '34.5', 34, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[PercentToLocalizedStringTransformer::INTEGER, 0, '34.4', 34, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[PercentToLocalizedStringTransformer::INTEGER, 0, '33.5', 34, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[PercentToLocalizedStringTransformer::INTEGER, 0, '32.5', 32, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.46', 3.5, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.45', 3.4, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.44', 3.4, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.35', 3.4, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.25', 3.2, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[null, 0, '34.6', 0.35, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[null, 0, '34.5', 0.34, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[null, 0, '34.4', 0.34, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[null, 0, '33.5', 0.34, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[null, 0, '32.5', 0.32, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[null, 1, '3.46', 0.035, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[null, 1, '3.45', 0.034, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[null, 1, '3.44', 0.034, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[null, 1, '3.35', 0.034, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
[null, 1, '3.25', 0.032, PercentToLocalizedStringTransformer::ROUND_HALF_EVEN],
// round halves (.5) away from zero
[PercentToLocalizedStringTransformer::INTEGER, 0, '34.6', 35, PercentToLocalizedStringTransformer::ROUND_HALF_UP],
[PercentToLocalizedStringTransformer::INTEGER, 0, '34.5', 35, PercentToLocalizedStringTransformer::ROUND_HALF_UP],
[PercentToLocalizedStringTransformer::INTEGER, 0, '34.4', 34, PercentToLocalizedStringTransformer::ROUND_HALF_UP],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.46', 3.5, PercentToLocalizedStringTransformer::ROUND_HALF_UP],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.45', 3.5, PercentToLocalizedStringTransformer::ROUND_HALF_UP],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.44', 3.4, PercentToLocalizedStringTransformer::ROUND_HALF_UP],
[null, 0, '34.6', 0.35, PercentToLocalizedStringTransformer::ROUND_HALF_UP],
[null, 0, '34.5', 0.35, PercentToLocalizedStringTransformer::ROUND_HALF_UP],
[null, 0, '34.4', 0.34, PercentToLocalizedStringTransformer::ROUND_HALF_UP],
[null, 1, '3.46', 0.035, PercentToLocalizedStringTransformer::ROUND_HALF_UP],
[null, 1, '3.45', 0.035, PercentToLocalizedStringTransformer::ROUND_HALF_UP],
[null, 1, '3.44', 0.034, PercentToLocalizedStringTransformer::ROUND_HALF_UP],
// round halves (.5) towards zero
[PercentToLocalizedStringTransformer::INTEGER, 0, '34.6', 35, PercentToLocalizedStringTransformer::ROUND_HALF_DOWN],
[PercentToLocalizedStringTransformer::INTEGER, 0, '34.5', 34, PercentToLocalizedStringTransformer::ROUND_HALF_DOWN],
[PercentToLocalizedStringTransformer::INTEGER, 0, '34.4', 34, PercentToLocalizedStringTransformer::ROUND_HALF_DOWN],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.46', 3.5, PercentToLocalizedStringTransformer::ROUND_HALF_DOWN],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.45', 3.4, PercentToLocalizedStringTransformer::ROUND_HALF_DOWN],
[PercentToLocalizedStringTransformer::INTEGER, 1, '3.44', 3.4, PercentToLocalizedStringTransformer::ROUND_HALF_DOWN],
[null, 0, '34.6', 0.35, PercentToLocalizedStringTransformer::ROUND_HALF_DOWN],
[null, 0, '34.5', 0.34, PercentToLocalizedStringTransformer::ROUND_HALF_DOWN],
[null, 0, '34.4', 0.34, PercentToLocalizedStringTransformer::ROUND_HALF_DOWN],
[null, 1, '3.46', 0.035, PercentToLocalizedStringTransformer::ROUND_HALF_DOWN],
[null, 1, '3.45', 0.034, PercentToLocalizedStringTransformer::ROUND_HALF_DOWN],
[null, 1, '3.44', 0.034, PercentToLocalizedStringTransformer::ROUND_HALF_DOWN],
];
}

/**
* @dataProvider reverseTransformWithRoundingProvider
*/
public function testReverseTransformWithRounding($type, $scale, $input, $output, $roundingMode)
{
$transformer = new PercentToLocalizedStringTransformer($scale, $type, $roundingMode);

$this->assertSame($output, $transformer->reverseTransform($input));
}

public function testReverseTransformEmpty()
{
$transformer = new PercentToLocalizedStringTransformer();
Expand Down

0 comments on commit 06a1a1b

Please sign in to comment.