Custom-set has been crippled by pull request #487

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.

@Eckankar Thanks for posting.
@m-dango Thoughts?

Please take a look at the following PR and let me know if it fits the structure you’re looking for: Use objects in custom-set exercise by m-dango · Pull Request #519 · exercism/perl5 · GitHub

There were several reasons for the change, the most important being the existing exercise had potential problems running in the online test runner. On the plus side, in the exercises current state, reworking an exercise to fit a desired structure takes a lot less effort.

1 Like

Thank you for addressing my concerns.

I have left some feedback on the PR. Overall, I think it looks much better; and with a little hint towards coerce, I think it should be pretty approachable.