Shlomi Fish wrote: > Hi Dov! > > On Wednesday 21 Oct 2009 16:46:51 Levenglick Dov-RM07994 wrote: > >> Hi, >> Thanks for the comments. >> >> 1. I will add use warnings. I don't think that it is worth the time to >> investigate if it runs on 5.005 or below. I'll add that as well. >> 2. I am >> not happy nor unhappy, just unaware. >> 3. I was unaware. I'll do that >> 4. Damn IDE! >> 5. The function is public and I didn't see the need to limit the user >> 6. Please explain. I don't understand >> 7. Why is regex injection bad? I use it for matching the files. >> >> > > I find it very hard to reply to that because my comments are in that order in > the bottom of the document quoted below, and I'll need to scroll up and down > or open them in a new window. Or copy&paste them in your reply to the place you consider most appropriate and then reply. Probably wouldn't've taken much longer than writing the reply you did write. Y'know, the edicai-shenal one. > This is a good proof for why top-posting is > evil: > Doesn't prove a damn thing and evil is much too strong a word for this. A minor inconvenience for some people at worst. > http://en.wikipedia.org/wiki/Posting_style > > So please reply again and this time quote the message properly. If it's not > possible using your mailer, you can use such standards-compliant mailers as > Mozilla's Thunderbird (cross-platform), GNOME Evolution or KDE 4's KMail > (Unix/Mac OS X-only I think, though it may a (crashy and quirky - ?) windows > port). > > Regards, > > Shlomi Fish > > >> Best Regards, >> Dov Levenglick >> SmartDSP OS Development Leader >> >> -----Original Message----- >> From: Shlomi Fish [mailto:[email protected]] >> Sent: Wednesday, October 21, 2009 15:30 >> To: [email protected] >> Cc: Levenglick Dov-RM07994 >> Subject: Re: [Israel.pm] File::Find::Recursive >> >> On Wednesday 21 Oct 2009 10:34:12 Levenglick Dov-RM07994 wrote: >> >>> Hi, >>> I wrote a module (File::Find::Recursive) that is supposed to search for >>> files matching match criteria and not matching ignore criteria in >>> folders with match and ignore criteria. It'll call callback functions for >>> matching files. >>> >>> I wrote it as a complimentary to File::Find::Rule which I found to be too >>> ambiguous. >>> >>> Any input is welcome. I will upload to CPAN as soon as the discussion >>> (hopefully) winds down and the namespace is granted. >>> >>> The module can be viewed at: http://pastebin.com/m7cfc397a >>> >> Well, first of all, I should note that pastebin.com does not >> syntax-highlight this file correctly, which makes it hard to read. Here >> are some notes: >> >> 1. You have "use strict;" but don't seem to have "use warnings;". Do you >> expect this module to work on perl-5.005 and below? >> >> 2. I see you have created File::Find::Simple -a simple alternative to >> File::Find. Why are you unhappy from: >> >> * http://www.shlomifish.org/open-source/projects/File-Find-Object/ >> >> Or possibly: >> >> * http://search.cpan.org/dist/File-Next/ >> >> (Which still has some of File-Find's philosophical limitations like being >> unable to be instantiated.). >> >> Naturally, one should note that F-F-O is a project I took over from its >> originator (Nanardon) and did most of my work on lately (and blogged about >> it). Lately, I've started working on File-Find-Object-Rule (a fork of File- >> Find-Rule to adapt it to File-Find-Object, which required quite a few >> changes and broke the external interface) and porting some of F-F-R's >> plugins to it. >> >> 3. According to PBP - all functions should have an explicit return at the >> end. >> >> 4. You seem to mix tabs and spaces inconsistently. >> >> 5. <<<< my ($self, $attr, @val) = @_; >>>> - @val is better passed as an >> array ref (though it's a topic of much heated debate). PBP seems to agree >> with me on it. >> >> 6. You seem to also invent another attribute module. Why can't you use >> Class-XSAccessor , Moose or possibly even Class-Accessor? >> >> 7. You have: >> >> <<<< >> next if grep /$file/, @{$self->{"_IGNORE_FILE_PATTERN"}}; >> >> >> Shouldn't you use a hash here instead, or at least \Q and \E ? This code >> smells of regex code injection (similar to SQL injection and XSS/HTML- >> injection). >> >> -------------------------- >> >> That's all I could find for now. >> >> Regards, >> >> Shlomi Fish >> >> >>> Best Regards, >>> Dov Levenglick >>> SmartDSP OS Development Leader, >>> DevTech, Technology and System Organization >>> Freescale Semiconductor Israel >>> Tel. +972-9-952-2804 >>> The information contained in this email is classified as: >>> [ ] Freescale General Business Information >>> [ ] Freescale Internal Use Only >>> [ ] Freescale Confidential Proprietary >>> [x] Personal Memorandum >>> SAVE PAPER - THINK BEFORE YOU PRINT >>> >>> _______________________________________________ >>> Perl mailing list >>> [email protected] >>> http://mail.perl.org.il/mailman/listinfo/perl >>> > >
_______________________________________________ Perl mailing list [email protected] http://mail.perl.org.il/mailman/listinfo/perl
