Re^2: Removing doubles and printing only unique values

[holli]

Sub-optimal.

say for do {                    # <-- mixed paradigms --------
   my %seen;                    # <-- reinventing the wheel  |
   grep { not $seen{ $_ } ++ }  # <---------------------------        
+                 
   map  { ( split m{;} )[ 1 ] } # too much logic, but that's nitpickin
+g
   <$inFH>;
   };'

Better? I think so:

use List::Util qw(uniqstr);
say join "\n", 
   uniqstr
   map  { $_->[0] }
   map  { [ split m{;} ] }
   <$inFH>;

You could of course roll your own implementation of uniq if you want to.

[johngg]

Coding styles are somewhat subjective so the question of better or worse is hard to answer. I will set out why I have coded the solution that way and, incidentally, point out that your code should be operating on the second field and not the first.

• say for do { ... I use a do block so that the %seen hash is not left hanging around but goes out of scope at the end of the block. Not necessary here as it is a one-liner but a good habit to get into I think. I'm not sure what you mean by "mixed paradigms" as I'm a bit old school, paradigms hadn't been invented when I started programming. To me it is just a piece of code that DWIMs.

• The use of my %seen; grep { not $seen{ $_ } ++ } ... is so simple that it hardly seems worth loading a module, especially as in this case the module uses pretty much the same wheel. I'm all for modules when a task is more complex but not when there is nothing to gain.

• I find it a bit puzzling that you consider map  { ( split m{;} )[ 1 ] } to be too much logic yet suggest as an alternative the use of two maps, the first to split and pass on an anonymous array, the second to pull out an element of that array (which actually should be element [1] not [0]). To me that appears to add more complication.

Thank you for making those suggestions (++), it was interesting to look again at my post in the light of your comments and question whether it was optimal or not. On balance I don't think I would change anything as I can justify to myself the reasons for coding it that way. I would be interested to know if others feel that my reasoning is flawed.

[holli]

Maybe I was overly critical there. It's just the purist in me who says "Don't mix iterative and functional style".