http://qs321.pair.com?node_id=607001


in reply to Should I leave behind beautiful code or readable code?

Ok, I got the point that the code is not as beautiful as I thought at first, given all those references to "Beauty is in the eye of the beholder." :-)
If somebody else had written my post, I myself would have probably replied along the lines of grinder's post. I also perfectly agree with BrowserUk. After all, I am supposed to write Perl code and use Perl's features and not to write C code in Perl.
I think I will slightly refactor the code as follows:
return map(split(' ',uc,2),@somearray);
Thanks jwkrahn for the hints.
Thanks everybody for your replies.
I love posting my meditations. Reading what others think is enlightening!

Max

Replies are listed 'Best First'.
Re^2: Should I leave behind beautiful code or readable code?
by perrin (Chancellor) on Mar 28, 2007 at 15:11 UTC
    That's actually harder to read than the original.
      "Surely you're joking, Mr. perrin!" ;)

      But since it looked like you might be serious, I have to wonder: in what sense could two chained "map" operators be easier to grok than a single "map"? (Like other replies in this thread, I'll admit I had to pause and think more than I would have liked in order to understand the snippet in the OP.)

      One other point that I haven't seen raised yet: if there is a concern about "readability for the sake of maintainability", one should perhaps consider how well the code plays with the perl debugger. I've seen a few threads here at PM where people have remarked on how certain constructs are difficult to debug, because there's no way to break at a reasonable point, or even to step through a section of code effectively.

      For the particular code snippet in the OP, some unexpected input -- e.g. in some random element of @somearray, all the whitespace turns out to be "\xA0" ( ) instead of "\x20" (space) -- might make for a tough problem to diagnose in the code as written, and I, as a maintainer, would be inclined to change it (e.g. assign results of first map to an array, assign results of second map to an array, then return that array) in order to look at intermediate results and have some hope of figuring out what's going wrong.

      Okay, that's not such a big deal. Go ahead and be compact, and in that regard, "simpler" (avoiding unnecessary looping operations) is better.

        It's not the chained maps that are problematic. It's the magic use of uc buried in the arguments to split, coupled with the use of map EXPR, ARRAY instead of map {...} ARRAY

        Personally, I'd prefer to see:

        return map { split / /, uc $_, 2 } @somearray

        I could argue both sides on the explicit return, but I come down in favour of being explicit because you're producing what Kent Beck calls an 'interesting return value'. The use of a block rather than a simple expression clues the reader in to the fact that the call to split isn't as simple as it looks. The explicit $_ argument to uc is there for clarification too.

        Frankly, I don't care about the maintainance programmer who's going to be here when I leave, I'm more worried about the psychopath who knows where I live. By paying the price of a few extra characters I get code that I know I won't be scratching my head over tomorrow, and that's the kind of peace of mind I like.

        But, if you want to play golf, map{split/ /,uc,2}@somearray looks like a winner.

        I realize that some perl coders can't seem to get out of bed in the morning without writing 5 map statements. The map is not that gratuitous here. The arrangement of the code though, with the buried uc() and the odd parentheses, is confusing. Here's a somewhat easier to read re-formatting:
        return map { split(' ', uc $_, 2) } @somearray;
        I might also break this into two, because the in-line makes the uncommon 3rd arg to split harder to pick up when scanning.
        my @separated = map { split(' ', $_, 2) } @somearray; @separated = map { uc $_ } @separated;
        Yes, it's probably slower. I don't care about the odd millisecond if it's easier to read. Some decent variable names would also make a big difference here.