When pairing with another programmer, I wrote some code which he described as "clever". Here it is.
sub xpc_namespaces {
my $self = shift;
my $values = $self->{xpc_namespaces};
my %namespaces = map { reverse %$_ } values %$values;
return \%namespaces;
}
If someone refers to my code as "clever", I should consider that a code smell and look for a better way of writing it. I know what that code does and why it does it, but I think that's because I wrote it.
I noticed the useless variable, so I thought about tightening it, but realized the logical conclusion was this:
sub xpc_namespaces {
{ map { reverse %$_ } values %{ shift->{xpc_namespaces} } };
}
That's just, well, wrong. Because of the structure of our XML, it's guaranteed we won't have collisions, but it's still an atrocity. Unless you have no choice, code should be written for programmers, not computers (SICP for the win!).
sub xpc_namespaces {
my $self = shift;
my @mapping = values %{ $self->{xpc_namespaces} };
my %namespaces = map { reverse %$_ } @mapping;
return \%namespaces;
}
Re:A question of factoring
slanning on 2008-11-25T15:44:53
I'm not even sure what part was clever. I was thinking it was the reversal of the hash, turning keys to values and vice-versa.Re:A question of factoring
Aristotle on 2008-11-25T17:11:49
It’s not easily apparent that the code is dealing with a HoH and the map expression reverses reading direction twice. Consider if it were possible to write something like this:
my %namespaces
= shift->{xpc_namespaces}
->values
->map( sub { %{$_}->reverse } )(This is not quite possible in this way even with autobox.)