Beefy Boxes and Bandwidth Generously Provided by pair Networks
Perl: the Markov chain saw
 
PerlMonks  

How do I un-map this code?

by Plankton (Priest)
on Mar 21, 2008 at 17:31 UTC ( #675484=perlquestion: print w/ replies, xml ) Need Help??
Plankton has asked for the wisdom of the Perl Monks concerning the following question:

Dear Monks,

I have also tried to avoid the use of map because I really can never understand what it is doing therefore assume that other people that might have to read my code might also find it difficult to understand if they ever had to maintain it.

Unfortunately I am the one maintaining the code of some really smart guy that thinks map, and unlesses and other crappy obfuscations make for maintainable code. Please forgive my rant ... anyways here's the bit of code I would to un map so I can add some debug to it.

sub _file_containing { my($self, $id) = @_; opendir(my $dir, $self->{update_dir}) or return; my @files = map { "$self->{update_dir}/$_" } sort { $a <=> $b } grep { /^\d+$/ && $_ <= $id } readdir $dir; return $files[-1]; }

Oh here's some more ranting ... sorry

I understand that back in the day structured programming it was a good idea to put an '_' in front of function if you where writing libraries so you wouldn't clutter the namespace of people using your libraries. But when I see it in circumstances like object orientated code is being used this practice of _my_great_subroutine only points out the pretensiousness of the doofus that wrote the code!

Thank you and I apologize for the rants.

Comment on How do I un-map this code?
Download Code
Re: How do I un-map this code?
by jhourcle (Prior) on Mar 21, 2008 at 17:43 UTC
    sub _file_containing { my($self, $id) = @_; opendir(my $dir, $self->{update_dir}) or return; my @files = sort { $a <=> $b } grep { /^\d+$/ && $_ <= $id } readd +ir $dir; return undef if !@files; return "$self->{update_dir}/$files[-1]"; }

    ... I don't know that it's particularly more legible, though.

    Oh -- and the '_' prefix tends to be used in OO code to mark private methods.

      Thanks that is a little more readable. I can't see how adding map to the mess would make it more readable. And as far as '_' meaning private ... I guess that would be private by convention since Perl doesn't really have any private methods or members, but conventions are only good if to tell people about them and this particular convention isn't documented anywhere. It might more sense to let people on that this method is private like so ...
      sub private_my_unstuckup_subroutine { ...
      ... or how about even ...
      # this method should be treated as though it is priavte sub my_unstuckup_method { ...
      ... or one could write the code in a language that actually supports private methods and memebers.
        I can't see how adding map to the mess would make it more readable.

        It removes the need for (useless) temporary variables and automatically handles the case of empty lists without any special checking.

        And as far as '_' meaning private ... I guess that would be private by convention since Perl doesn't really have any private methods or members, but conventions are only good if to tell people about them and this particular convention isn't documented anywhere.
        On the contrary, this widely used naming convention is documented in the core Perl docs, the Camel book, and in many other places:
        • From Perl style: "You can use a leading underscore to indicate that a variable or function should not be used outside the package that defined it."
        • From Perl Best Practices, Chapter 3 Naming Conventions: 3.10 "Prefix "for internal use only" subroutines with an underscore."

Re: How do I un-map this code?
by Plankton (Priest) on Mar 21, 2008 at 18:18 UTC
    I guess what is going on in this code could be replaces with a simple shell command ...
    $ grep -l $id $dir/*
    Maybe I could alias grep to _grep to make it private! LOL!
      Ah, no. The code in the OP is looking for a file name that matches some criteria (the maximum numerical file less than or equal to some id). Your code is looking for files with contents that match some criteria (contains some string). So the original function does seem poorly named. But if you really do hate want to avoid perl that much, you could (in ksh):
      cd $dir file=$dir/$(ls +([[:digit:]]) | awk "\$0 <= $id" | sort -rn | head -1)
      or if you wanted to even avoid awk (after all, it resembles perl a bit):
      file=$dir/$(ls +([[:digit:]]) | while read line do (( line <= id )) && print $line done | sort -rn | head -1)
      But to "fix" the original code, I'd probably just add something like this before the opendir or before the function itself:
      # Find the maximum filename less than or equal to id
Re: How do I un-map this code?
by dragonchild (Archbishop) on Mar 21, 2008 at 18:19 UTC
    I have also tried to avoid the use of map because I really can never understand what it is doing

    I translate this statement as "I'm scared, so I refuse to address my fear." If you really don't want to address your fear, stop now. I plan on getting you to understand map so that you never have to worry again.

    map, grep, and sort all operate the exact same way. They take a list of stuff and return a list of stuff.

    • sort takes the list and sorts it, returning back every item that was in the original list, but possibly in a different place.
    • grep takes the list and applies a function to each item. If the function returns true, that item is in the result list. If the function returns false, that item is skipped. The items that make it are in the same order as they were originally.
    • map takes the list and applies a function to each item. It takes the return value from the function and puts it into the result list. The items are returned in the same order.

    So, an example:

    my @l = ( 5, 4, 3, 2, 1 ); my @sorted = sort { $a <=> $b } @l; # Sorted is ( 1, 2, 3, 4, 5 ) my @grepped = grep { $_ > 2 } @l; # Grepped is ( 5, 4, 3 ) my @mapped = map { $_ * 2 } @l; @ Mapped is ( 10, 8, 6, 4, 2 )

    Now, the biggest problem people usually have with map and grep (though, funnily enough, not sort) is that you have to read them from right to left. This is in direct opposition to how you read everything else (which is from left to right). Perl 6 will provide a mechanism where you can have map, grep, and other list operators that can be read from left to right. It will look something like:

    my @l = ( 5, 4, 3, 2, 1 ); @l ==> grep { $_ > 2 } ==> sort { $a <=> $b } ==> map { $_ * 2 } ==> @final; # Final contains ( 6, 8, 10 )
    Is that easier to read?

    Oh, and fearing unless is just plain old laziness - the bad kind. unless(...) is exactly identical to if(!(...)). Nothing less, nothing more. Feel free to revise as desired.


    My criteria for good software:
    1. Does it work?
    2. Can someone else come in, make a change, and be reasonably certain no bugs were introduced?
Re: How do I un-map this code?
by planetscape (Canon) on Mar 21, 2008 at 18:47 UTC
Re: How do I un-map this code?
by eric256 (Parson) on Mar 21, 2008 at 19:49 UTC

    I would format it slightly, and then just read from the bottum up. Yes map and grep can be scary, but normaly reading them isn't hard (just figuring out how/when to use them is.

    sub _file_containing { my($self, $id) = @_; opendir(my $dir, $self->{update_dir}) or return; my @files = map { "$self->{update_dir}/$_" } sort { $a <=> $b } grep { /^\d+$/ && $_ <= $id } readdir $dir; return $files[-1]; }

    So now start at the bottom of the statment and read up. readdir $dir takes all the files in the directory $self->{update_dir} and pipes them into the grep above it. grep { /^\d+$/ && $_ <= $id } is a filter that only lets files that look like a number and are less than $id through. sort { $a <=> $b } sorts the files using a numeric sort (instead of cmp that would do an alphanumeric). map  { "$self->{update_dir}/$_" } takes that list and adds the actual directory to the front of each element. The map in this case is kinda pointless because you then return only the last element, however the map means that you don't have to check if there was indeed a last element in the list to return. If you replaced the last return with return $self->{update_dir} || '/' || $files[-1]; then you would wrongly return the directory itself when there where no matching files.


    ___________
    Eric Hodges
Re: How do I un-map this code?
by jwkrahn (Monsignor) on Mar 21, 2008 at 21:16 UTC

    Because the function is only returning one item from the list there is no reason to iterate through and sort the entire list:

    sub _file_containing { my ( $self, $id ) = @_; opendir my $dir, $self->{ update_dir } or return; my $ret; while ( my $file = readdir $dir ) { next if $file !~ /^\d+$/ || $file > $id; $ret = $file if $ret < $file; } return "$self->{update_dir}/$ret"; }
Re: How do I un-map this code?
by ikegami (Pope) on Mar 21, 2008 at 22:22 UTC
    my @list = map BLOCK LIST;
    can be written as
    my @list; for (LIST) { push @list, do BLOCK; }

    So

    my @files = map { "$self->{update_dir}/$_" } sort { $a <=> $b } grep { /^\d+$/ && $_ <= $id } readdir $dir;

    could be written as

    my @files; for ( sort { $a <=> $b } grep { /^\d+$/ && $_ <= $id } readdir $dir ) { push @files, "$self->{update_dir}/$_"; }

    If what you have a problem with is really the chaining of all those functions and not map specifically, just reverse the order and use a temporary array to hold the arguments.

    So

    my @files = map { "$self->{update_dir}/$_" } sort { $a <=> $b } grep { /^\d+$/ && $_ <= $id } readdir $dir;

    could be written as

    my @temp; @temp = readdir $dir; @temp = grep { /^\d+$/ && $_ <= $id } @temp; @temp = sort { $a <=> $b } @temp; @temp = map { "$self->{update_dir}/$_" } @temp; my @files = @temp;
Re: How do I un-map this code?
by runrig (Abbot) on Mar 22, 2008 at 18:30 UTC
    use List::Util qw(max); my($self, $id) = @_; my $dir = $self->{update_dir}; opendir(my $dir_h, $dir ) or return; #Change this if you expect a possible valid id of "0" my $id = max( grep { /^\d+$/ && $_ <= $id } readdir $dir_h ) or return; return "$dir/$id";

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://675484]
Approved by kyle
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others meditating upon the Monastery: (7)
As of 2014-12-25 09:48 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    Is guessing a good strategy for surviving in the IT business?





    Results (159 votes), past polls