Roy Corey (xerhino) wrote in perl,
Roy Corey
xerhino
perl

  • Mood:

Good to be bad.

Ok, this isn't something I would do in code that anyone would ever have to support or modify. This was purely a temporary monitoring script for an ill concieved application that, for dubious reasons, had 30 duplicate directories where logs could be written, depending on the day/time/traffic/phase of the moon/etc.

my @logs = grep {-e $_} map { "/opt/foo/bar/$_/logs/date.log" } 1..30;

I got a map, a grep, and a range operator all on one line and it returns an array of all the log files in the appropriate spots that actually exist. After so long making everything painfully clear this feels decadent. Really, I don't think it's that bad, but I guarentee no one else on my team would understand it. I just had to share with a group that would.

Note: I follow most of the tenets of Damian Conway's "Perl Best Practices" in my professional scripting.
Subscribe
  • Post a new comment

    Error

    default userpic
    When you submit the form an invisible reCAPTCHA check will be performed.
    You must follow the Privacy Policy and Google Terms of use.
  • 27 comments

Deleted comment

Thanks. Yep, I commented as a matter of habit. I hope commenting code is like using the turn signal. If you're good you do it even when no one is looking.

rekoil

9 years ago

xerhino

9 years ago

rekoil

9 years ago

londo

9 years ago

fyremoon

9 years ago

xerhino

9 years ago

Deleted comment

xerhino

9 years ago

If they don't understand something as straightforward as that, you should be able to make their heads explode with a Schwartzian Transform.
Or an "orcish maneuver" . :-)
Along the same lines, the ugliest regexp I've ever written:

my ($user, $host, $pty, $datetime, $idle, $idlehour, $idlemin, $machine) = m|^(\w+)\s+([\w.]+):([\w\/\%]+)\s+(\w{3}\s+\d{1,2}\s+\d\d:\d\d)\s+((\d*:)*:?(\d*))\s*\(?(.*)\)?\s*\)$|;

It's for parsing the output of rusers. I wrote it years ago, so it might be awfully written (beyond the mere fact of its unreadability). Definitely write-only code, even for its author!
Yikes! Actually I was hoping to see some other examples of surreptitious perl, so this is good. One thing though, regexps are notoriously difficult. Yeah, this is a long ago example, but I'll say that the "/x" flag is, in this case, everybody's friend.

ruakh

9 years ago

xerhino

9 years ago

ruakh

9 years ago

I tend to not use the range operator, but that's mostly because it hasn't much applied to things I've done; otherwise it's terribly common to mix map and grep on a line with any source array. I really only have two complaints with the line of code: the hard-coded path, and the use of -e.

I hope I don't have to explain my aversion to hard-coded paths. The problem with -e is that it's not clear that it's not clearly the right test (and this is generally a problem with -X in Perl): should it be -e, -f, -s? I trust stat() and friends more, and the more obscure the -X test the more I distrust it.
Yeah, I share your aversion to hard coded paths. As far as the -e test, I considered -r, etc. Really I only cared if the log existed since, in this trivial and temporary case, I knew the logs would all be readable and regular. Not the most robust test, but enough for the quick fix.
Anything wrong with


my @logs = glob("$prefix/[0-9]*/logs/date.log");


It isn't exactly the same, of course, but it's close.
I'd go for this one as well, as it performs less file testing.

xerhino

9 years ago

Wanna get more decadent?

Drop the $_ from the -e, since it's assumed.

my @logs = grep {-e} map { "/opt/foo/bar/$_/logs/date.log" } 1..30;
Aha. You know I was wondering about that and didn't get around to testing it o make sure. Thanks.
Ok, so setting aside the valid points made about -e and the hardcoded path, why wouldn't you consider putting this in production code and why wouldn't your cow orkers be able to understand it?

I am being serious. This is simple, self-documenting code. Perl borrows heavily from languages and paradigms that do things well. List transformation (map) and list filtering (grep) have been borrowed from functional languages. It allows you to do amazing things without saying a lot. This is what good programmers should be doing.

I would have admonished a cow orker in a peer code review if I had seen:

# Create a list of possible directory names
my @dir = 1 .. 30;

# Create a list possible absolute file names (unix specific)
my @path = map { "/opt/foo/bar/$_/logs/date.log" } @dir;

# Filter out any file paths that do not exist
my @log = grep {-e $_} @path;

I would have been even more frustrated if they had avoided map and grep all together because they felt they were too advanced and opted in favor of for loops

my @path;
for (@dir) {
    push @dir, "/opt/foo/bar/$_/logs/date.log"
}

my @log;
for (@path) {
    next if ! -e $_;
    push @log, $_;
}


While I also don't see any issue with the glob solution it still requires filtering out the non-existant paths.

What exactly is so advanced about that line that is un(read|maintain|understand)able?
Dude, don't make me cry. If I worked somewhere that I could use code like that (with comments as to the crazy structure it applied to) I would love it. You ask why my co-workers wouldn't be able to understand it: because they aren't what I would call 'programmers'. Yeah, they do program software, but it's all very straight forward. I don't trust a single sql statement they might write. In fact, if I see that they wrote a query by themselves I begin to sweat and nervously check the database.

So, basically, there are limited places one can write tight and intelligent code. If I saw that statement with no comments, which I don't believe you are advocating, I would be angry with the original programmer and I would have to mentally run through the operation to figure out why it was creating a bunch of hypothetical directories. If I saw it with some explanation I would be more than happy. I'd have no problem with a functional and dense bit of code as long as some relatively clear explanation of the twisted purpose followed.

There is a reason why I had a rubber stamp made with the text "written by monkeys".