php-ds / ext-ds

An extension providing efficient data structures for PHP 7
https://medium.com/p/9dda7af674cd
MIT License
2.11k stars 95 forks source link

Any reason why Set doesn't have a map method? #110

Closed enumag closed 6 years ago

enumag commented 6 years ago
public Ds\Set Ds\Set::map ( callable $callback )

It's not even listed in #90. Is there some reason why this method can't or shouldn't be added?

rtheunissen commented 6 years ago

What would you expect it to return? It can't be a Set unless you suggest that there might some mapped values that won't be included in the result set (as there could be duplicates).

Eg.

$s = new \Ds\Set();
$s[] = 1;
$s[] = 2;
$s[] = 3;

$s->map(function($value) {
    return $value % 2;
});

print_r($s);
Ds\Set Object
(
    [0] => 1
    [1] => 0
)
teohhanhui commented 6 years ago

map returns a Map. Makes perfect sense, no? :laughing:

/half kidding

rtheunissen commented 6 years ago

I kind of like the idea of map resulting in a Set where duplicates are ignored, like shown above.

TheDigitalOrchard commented 6 years ago

+1 for adding a map() method. It makes perfect sense to mirror array_map functionality in these DS classes, where appropriate.

It should return a Set (not a Map) since it's merely a filtered/processed full or subset of itself, much like array_map() accepts an array and returns an array. It does act on the passed array directly.

array_map() returns an array containing all the elements of array1 after applying the callback function to each one. The number of parameters that the callback function accepts should match the number of arrays passed to the array_map()

But unlike @rtheunissen's example code, it should return a new Set instead of modifying by reference, I think, as shown here:

$s1 = new \Ds\Set();
$s1[] = 1;
$s1[] = 2;
$s1[] = 3;

$s2 = $s1->map(function($value) {
    return $value % 2;
});

print_r($s2);

I'm on the fence about returning a Set or modifying the original. Many PHP functions are inconsistent in their own design, with some returning and some modifying by reference.

EDIT - Now that I think about it, I'd want the original Set to be modified. I'm calling a method on an object. I expect that method to act on that object. If I wanted a copy, I'd just copy the object.

rtheunissen commented 6 years ago

But unlike @rtheunissen's example code, it should return a new Set instead of modifying by reference

Yeah this was my bad - map should definitely return a mapped copy. It would be very unusual for methods like map and filter to mutate the instance. If you wanted a map that mutates, I'd rather implement something like apply that does that.

teohhanhui commented 6 years ago

No, it should definitely not modify the original.

rtheunissen commented 6 years ago

No, it should definitely not modify the original.

That's what I'm saying, yes. 👍

simPod commented 4 years ago

Any reason for closing this? I just found myself array_mapping Set->toArray() several times this week already. I think the FR is still viable, no?