Since we aren’t allowed to comment on the GitHub repo, I’ll repost my post here:
I was quite surprised today to see that custom-set
has been completely changed by pull request #487, under the guise of “generating full tests”.
This change has (in my opinion) completely ruined the point of the exercise.
The description of the problem says:
Sometimes it is necessary to define a custom data structure of some type, like a set.
In this exercise you will define your own set.
How it works internally doesn’t matter, as long as it behaves like a set of unique elements.
Before this change, the task was to implement a set object, which could be implemented in any way one wanted. The obvious choice for this would be to represent the set using a hashref mapping each element to the value 1.
After this change, the problem doesn’t operate on objects anymore - so any teaching points on learning about objects and encapsulation are lost.
Sets are also forced to be arrayrefs, which not only makes the solutions much uglier - it also makes them terribly inefficient.
In the reference implementation for intersection
, for instance:
sub set_intersection {
my ($set1, $set2) = @_;
my %set1 = (map {$_ => 1} @{$set1});
my %set2 = (map {$_ => 1} @{$set2});
return [ grep { $set1{$_} && $set2{$_} } keys %{ {%set1, %set2} } ];
}
Notice, how $set1
and $set2
both are converted into a more sensible representation - only for that representation to be discarded for the return value.
I don’t think the problem makes much sense in its current incarnation - the user doesn’t learn about doing proper encapsulation like the problem is intended to; and is forced to use a sub-optimal representation for their sets.
Personally, I’d like to see this change reverted, such that the problem makes sense again.
I don’t see any problem with the old test suite; and if there is one, it makes more sense to me to add the cases that are missing to that, rather than scrap the entire problem to start over.