I was working on some code the other day and ran across this little gem. Granted, I've simplified (and removed any proprietary information from) all the expressions, because they're much more verbose than this. But I was pretty incredulous when I saw it, considering I was supposed to modify it to change its functionality. The formatting is about the same, though, but the linebreaks might be in slightly different spots. The important bit is that it was actually much more unreadable than this. The formats actually came from object variables (i.e. $obj->var->{DIRECT_HASH_ACCESS} - other than the one hard coded near the beginning) which helped to clutter everything up, and the %hash keys (and name) were much longer. I also wanted to make it fit in the page without making it expand too wide :)
push @arr, sprintf( q(<div long format string >%s</div>),
join( '|', map { sprintf ( $_ eq $i ? 'formatA' :
'formatB', $variable ? $_ : ( $uri !~ /regex/) ?
join ( '/', 'something', $_ ) : join ( '/',
'something/else', $_), $hash{$_}) } ( sort {
$hash{$a} cmp $hash{$b} } ( keys %hash )) ));
If I ever come across the person who wrote this and left it as is, jumbled together and uncommented, I will punch them squarely in the face.
First, the lack of formatting makes it impossible to read. And there are superfluous parenthesis that help to clutter things up.
Then there's the superfluous use of the joins stuck in the middle. The joins don't do anything - I thought at first they might help avoid a "Use of uninitialized value in a concatenation" warning, but that can't happen if you're iterating over the keys of a %hash.
There also seems to be at least one of the ternaries I can avoid by doing a little work up front - it doesn't vary based on which %hash is currently being iterated over.
If you take care of those four things, it helps a bit, but overall it's still not all that great.
my $path = 'something';
if( $uri =~ /regex/ ){
$path .= '/else';
}
push @arr,
sprintf
q(<div long format string >%s</div>),
join '|',
map { sprintf $_ eq $i ? 'formatA'
: 'formatB',
$variable ? $_
: "$path/$_",
$hash{$_}
}
sort { $hash{$a} cmp $hash{$b} }
keys %hash;
I'm still not happy about the push sprintf join map sprintf ?: ?: sort keys %hash structure, and will probably end up breaking it into more readable chunks later. But at least now it's a little easier to follow - and most important, modify.
time passes
So I was able (according to the requirements) to rip out the differing functionality for various states, and as such was able to pull the ternaries out altogether.
my $path = 'something';
if( $uri =~ /regex/ ){
$path .= '/else';
}
push @arr,
sprintf
q(<div long format string >%s</div>),
join '|',
map { sprintf 'formatB',
"$path/$_",
$hash{$_}
}
sort { $hash{$a} cmp $hash{$b} }
keys %hash;
