moneyphp / money

PHP implementation of Fowler's Money pattern.
http://moneyphp.org
MIT License
4.56k stars 439 forks source link

Aggregate Currencies unexpected behaviour #749

Closed bakurin closed 1 year ago

bakurin commented 1 year ago

How the AggregateCurrencies class creates an integrator over the currencies might lead to bugs. The class iterates the nested Currencies object and appends each of its iterators to the AppendIterator object. The implementation is straightforward and works as expected. But an attempt to convert the iterator object to an array with the iterator_to_array function might lead to an unexpected result:


$list1 = new CurrencyList('EUR' => 2]);
$list2 = new CurrencyList('USD' => 2]),
$currencies = new AggregateCurrencies([
    $list1,
    $list2,
]);

$array = iterator_to_array($currencies); // there is only USD in the $array

In the example above $array variable contains only "USD". It happens because the second argument of iterator_to_array $preserve_keys is true by default and both elements form the $list2 override values from $list1

This behavior is totally explainable and can be fixed by setting the second argument of iterator_to_array to false. But IMHO it means that implementation details leak to client code.

It would be nice to get any other opinion on this before taking any further steps.

frederikbosch commented 1 year ago

This is the expected result, we tested iterator_to_array with the false parameter.

Rather than using iterator_to_array, I more often use this statement.

$array = [...$currencies];