Athari / YaLinqo

Yet Another LINQ to Objects for PHP [Simplified BSD]
https://athari.github.io/YaLinqo
BSD 2-Clause "Simplified" License
439 stars 39 forks source link

There aren't enough examples & union method doesn't work as expected #16

Closed ericgcc closed 8 years ago

ericgcc commented 8 years ago

The API is unusable if there aren't enough examples of the use of the methods. After many hours of trying to use the union() method, it doesn't generate the result expected in joining two simple arrays of strings because there are missing elements in the resulting array.

I don't know if i'm doing something wrong:

$local_types = ["MOBILE", "OFFICE", "WHATSAPP", "PRIVATE", "WORK", "MAIN"];
$remote_types = ["MOBILE", "RADIO", "WHATSAPP", "OFFICE"];

After executing:

Enumerable::from($local_types)->union($remote_types)->toArray();

The resulting array is:

"MOBILE",
"RADIO",
"WHATSAPP",
"PRIVATE",
"MAIN"

Where is the "OFFICE"?

Athari commented 8 years ago

Short version:

Use ToList instead of ToArray. Or just iterate with foreach.

Long version:

There're a couple of gotchas caused by presence of keys in PHP's iterators and the library trying to not lose any data, including keys.

Let's make keys in your arrays explicit:

$local_types = [0 => "MOBILE", 1 => "OFFICE", 2 => "WHATSAPP", 3 => "PRIVATE", 4 => "WORK", 5 => "MAIN"];
$remote_types = [0 => "MOBILE", 1 => "RADIO", 2 => "WHATSAPP", 3 => "OFFICE"];

Union method uses KeySelector argument to join arrays, by default it selects values. So the sequence produced is this (just for illustration purposes):

[0 => "MOBILE", 1 => "OFFICE", 2 => "WHATSAPP", 3 => "PRIVATE", 4 => "WORK", 5 => "MAIN", 1 => "RADIO"];

Note that there're two elements with the key "1". ToArray method creates an array, iterates over an iterator and puts key-value pairs into array as is. This causes "OFFICE" value to disappear. What you need is ToList method, which works just like ToArray, but discards keys from original iterators and generates sequental integer keys instead. So the produced array will be:

[0 => "MOBILE", 1 => "OFFICE", 2 => "WHATSAPP", 3 => "PRIVATE", 4 => "WORK", 5 => "MAIN", 6 => "RADIO"];

You also can avoid all these problems if you just use iterators without converting them to arrays. For example, if you iterate over an iterator with foreach, you'll get all items and avoid allocating unnecessary memory blocks.

foreach (Enumerable::from($local_types)->union($remote_types) as $type)
    echo($type);

Any "collapsing" methods like Max or ToString will work correctly too.

Iterators are less restricted than arrays: they can contain duplicate keys, objects in keys etc. If your transformation produces iterators like these, ToArray method may produce unexpected results.

By the way, documentation of ToArray includes this paragraph:

Keys from the sequence are preserved. If the source sequence contains multiple values with the same key, the result array will only contain the latter value. To discard keys, you can use {@link toList} method. To preserve all values and keys, you can use {@link toLookup} method.

Though the fact that Union method preserves keys of both sequences isn't that obvious I guess.

ericgcc commented 8 years ago

Thanks a lot for the well explained answer, I'll try the code.