[#ZF-11798] Zend_Currency constructor overwrites the display option if a locale defines a symbol - Zend Framework Issue Tracker

Details

Type: Bug
Status: Open
Priority: Major
Resolution: Unresolved
Affects Version/s: 1.11.10
Fix Version/s: None
Component/s: Zend_Currency
Labels:

Description

For a locale which defines a currency symbol, it's completely useless to define the display option because the constructor overwrites it with the Zend_Currency::USE_SYMBOL constant.

// Get the format
if (!empty($this->_options['symbol'])) {
    $this->_options['display'] = self::USE_SYMBOL;
}

So the following code doesn't work:

$currency = new Zend_Currency(array('value' => 100, 'display' => Zend_Currency::USE_SHORTNAME), 'de_DE');
print $currency;

Because this code prints always the currency symbol €.

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Adam Lundrigan added a comment - 28/Oct/11 7:05 PM

Reproducing unit test case (against locale en_US for convenience):

Index: tests/Zend/CurrencyTest.php
===================================================================
--- tests/Zend/CurrencyTest.php (revision 24537)
+++ tests/Zend/CurrencyTest.php (working copy)
@@ -826,4 +826,28 @@
         $currency->setService('Zend_Currency_Service');
         $this->assertTrue($currency->getService() instanceof Zend_Currency_Service);
     }
+
+    /**
+     * @group ZF-11798
+     * @dataProvider providerConstructorAllowsOverridingCurrencyDisplayFormat
+     */
+    public function testConstructorAllowsOverridingCurrencyDisplayFormat($display, $expected)
+    {
+        $currency = new Zend_Currency(array('value' => 100, 'display' => $display), 'en_US');
+        $this->assertEquals($expected, $currency->toString());
+    }
+
+    /**
+     * Data Provider for testConstructorAllowsOverridingCurrencyDisplayFormat
+     * @see ZF-11798
+     */
+    public function providerConstructorAllowsOverridingCurrencyDisplayFormat()
+    {
+        return array(
+            array(Zend_Currency::NO_SYMBOL, '100.00'),
+            array(Zend_Currency::USE_SYMBOL, '$100.00'),
+            array(Zend_Currency::USE_SHORTNAME, 'USD100.00'),
+            array(Zend_Currency::USE_NAME, 'US Dollar100.00')
+        );
+    }
 }

Result:

++ phpunit --verbose --group ZF-11798 AllTests
PHPUnit 3.5.14 by Sebastian Bergmann.

F.FF

Time: 8 seconds, Memory: 425.00Mb

There were 3 failures:

1) Zend_CurrencyTest::testConstructorAllowsOverridingCurrencyDisplayFormat with data set #0 (1, '100.00')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-100.00
+$100.00

/usr/local/apache2/htdocs/lib/zfdev/trunk/tests/Zend/CurrencyTest.php:837

2) Zend_CurrencyTest::testConstructorAllowsOverridingCurrencyDisplayFormat with data set #2 (3, 'USD100.00')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-USD100.00
+$100.00

/usr/local/apache2/htdocs/lib/zfdev/trunk/tests/Zend/CurrencyTest.php:837

3) Zend_CurrencyTest::testConstructorAllowsOverridingCurrencyDisplayFormat with data set #3 (4, 'US Dollar100.00')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-US Dollar100.00
+$100.00

/usr/local/apache2/htdocs/lib/zfdev/trunk/tests/Zend/CurrencyTest.php:837

FAILURES!
Tests: 4, Assertions: 4, Failures: 3.

Show

Adam Lundrigan added a comment - 28/Oct/11 7:05 PM Reproducing unit test case (against locale en_US for convenience):

Index: tests/Zend/CurrencyTest.php
===================================================================
--- tests/Zend/CurrencyTest.php (revision 24537)
+++ tests/Zend/CurrencyTest.php (working copy)
@@ -826,4 +826,28 @@
         $currency->setService('Zend_Currency_Service');
         $this->assertTrue($currency->getService() instanceof Zend_Currency_Service);
     }
+
+    /**
+     * @group ZF-11798
+     * @dataProvider providerConstructorAllowsOverridingCurrencyDisplayFormat
+     */
+    public function testConstructorAllowsOverridingCurrencyDisplayFormat($display, $expected)
+    {
+        $currency = new Zend_Currency(array('value' => 100, 'display' => $display), 'en_US');
+        $this->assertEquals($expected, $currency->toString());
+    }
+
+    /**
+     * Data Provider for testConstructorAllowsOverridingCurrencyDisplayFormat
+     * @see ZF-11798
+     */
+    public function providerConstructorAllowsOverridingCurrencyDisplayFormat()
+    {
+        return array(
+            array(Zend_Currency::NO_SYMBOL, '100.00'),
+            array(Zend_Currency::USE_SYMBOL, '$100.00'),
+            array(Zend_Currency::USE_SHORTNAME, 'USD100.00'),
+            array(Zend_Currency::USE_NAME, 'US Dollar100.00')
+        );
+    }
 }

Result:

++ phpunit --verbose --group ZF-11798 AllTests
PHPUnit 3.5.14 by Sebastian Bergmann.

F.FF

Time: 8 seconds, Memory: 425.00Mb

There were 3 failures:

1) Zend_CurrencyTest::testConstructorAllowsOverridingCurrencyDisplayFormat with data set #0 (1, '100.00')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-100.00
+$100.00

/usr/local/apache2/htdocs/lib/zfdev/trunk/tests/Zend/CurrencyTest.php:837

2) Zend_CurrencyTest::testConstructorAllowsOverridingCurrencyDisplayFormat with data set #2 (3, 'USD100.00')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-USD100.00
+$100.00

/usr/local/apache2/htdocs/lib/zfdev/trunk/tests/Zend/CurrencyTest.php:837

3) Zend_CurrencyTest::testConstructorAllowsOverridingCurrencyDisplayFormat with data set #3 (4, 'US Dollar100.00')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-US Dollar100.00
+$100.00

/usr/local/apache2/htdocs/lib/zfdev/trunk/tests/Zend/CurrencyTest.php:837

FAILURES!
Tests: 4, Assertions: 4, Failures: 3.

Hide

Permalink

Adam Lundrigan added a comment - 28/Oct/11 7:10 PM

YMMV, but this fixes the issue for me. I have to do more research before i'm comfortable suggesting it as a complete fix and free of BC implications:

Index: library/Zend/Currency.php
===================================================================
--- library/Zend/Currency.php   (revision 24537)
+++ library/Zend/Currency.php   (working copy)
@@ -91,6 +91,11 @@
      */
     public function __construct($options = null, $locale = null)
     {
+        $calloptions = $options;
+        if (is_array($options) && isset($options['display'])) {
+            $this->_options['display'] = $options['display'];
+        }
+
         if (is_array($options)) {
             $this->setLocale($locale);
             $this->setFormat($options);
@@ -118,12 +123,15 @@
             require_once 'Zend/Currency/Exception.php';
             throw new Zend_Currency_Exception("Currency '$options' not found");
         }
-
+
         // Get the format
-        if (!empty($this->_options['symbol'])) {
-            $this->_options['display'] = self::USE_SYMBOL;
-        } else if (!empty($this->_options['currency'])) {
-            $this->_options['display'] = self::USE_SHORTNAME;
+        if ((is_array($calloptions) && !isset($calloptions['display']))
+                || (!is_array($calloptions) && $this->_options['display'] == self::NO_SYMBOL)) {
+            if (!empty($this->_options['symbol'])) {
+                $this->_options['display'] = self::USE_SYMBOL;
+            } else if (!empty($this->_options['currency'])) {
+                $this->_options['display'] = self::USE_SHORTNAME;
+            }
         }
     }

Show

Adam Lundrigan added a comment - 28/Oct/11 7:10 PM YMMV, but this fixes the issue for me. I have to do more research before i'm comfortable suggesting it as a complete fix and free of BC implications:

Index: library/Zend/Currency.php
===================================================================
--- library/Zend/Currency.php   (revision 24537)
+++ library/Zend/Currency.php   (working copy)
@@ -91,6 +91,11 @@
      */
     public function __construct($options = null, $locale = null)
     {
+        $calloptions = $options;
+        if (is_array($options) && isset($options['display'])) {
+            $this->_options['display'] = $options['display'];
+        }
+
         if (is_array($options)) {
             $this->setLocale($locale);
             $this->setFormat($options);
@@ -118,12 +123,15 @@
             require_once 'Zend/Currency/Exception.php';
             throw new Zend_Currency_Exception("Currency '$options' not found");
         }
-
+
         // Get the format
-        if (!empty($this->_options['symbol'])) {
-            $this->_options['display'] = self::USE_SYMBOL;
-        } else if (!empty($this->_options['currency'])) {
-            $this->_options['display'] = self::USE_SHORTNAME;
+        if ((is_array($calloptions) && !isset($calloptions['display']))
+                || (!is_array($calloptions) && $this->_options['display'] == self::NO_SYMBOL)) {
+            if (!empty($this->_options['symbol'])) {
+                $this->_options['display'] = self::USE_SYMBOL;
+            } else if (!empty($this->_options['currency'])) {
+                $this->_options['display'] = self::USE_SHORTNAME;
+            }
         }
     }

People

Assignee:

Thomas Weidner

Reporter:

Christian Kaps

Vote (1)

Watch (1)

Dates

Created:

06/Oct/11 9:12 AM

Updated:

04/May/12 7:26 PM