Shlomi Fish wrote: > Hi Dov! > > See below for my inline response. > > On Thursday 22 Oct 2009 07:19:25 Levenglick Dov-RM07994 wrote: > >> You must be kidding! Here goes >> >> > > I was not. I am trying to educate people to send me properly formatted > E-mails > and may refuse to reply to them without the poster complying with proper > style > and formatting guidelines. See for example: > > http://zgp.org/pipermail/linux-elitists/2009-September/013185.html > > Book Publishers have tight typesetting rules, which yields more readable > rules. Human languages have proper rules for spelling, grammars, syntaxes, > punctuation, idiomatic writing, etc. which result in them being easier to > understand and digest. There are similar design decisions in GUI programs, > CLI > programs, programming languages, coding etc. > > Likewise, E-mail has some netiquette rules for formatting, sending and > replying that result in it being easier to read, and easier to understand. > You mean rules like - avoid criticizing people in a public forum ?
> >> Best Regards, >> Dov Levenglick >> SmartDSP OS Development Leader >> >> -----Original Message----- >> From: Shlomi Fish [mailto:[email protected]] >> Sent: Wednesday, October 21, 2009 20:05 >> To: [email protected] >> Cc: Levenglick Dov-RM07994 >> Subject: Re: [Israel.pm] File::Find::Recursive >> >> 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. This is a good proof for why >> top-posting is evil: >> >> 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? >>> >> [Dov L.] 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. >> >> > > The "use warnings;" pragma is not available in 5.005 and below - that's why I > asked. > > > >>> 2. I see you have created File::Find::Simple -a simple alternative to >>> File::Find. Why are you unhappy from: >>> >> [Dov L.] I am not happy nor unhappy, just unaware >> >> > > OK. It is a good idea to do a CPAN search before re-inventing yet another > wheel. > > >>> * 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. >>> >> [Dov L.] I was unaware. I'll do that >> >> > > Thanks. > > >>> 4. You seem to mix tabs and spaces inconsistently. >>> >> [Dov L.] Damn IDE! >> >> > > :-) > > >>> 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. >>> >> [Dov L.] The function is public and I didn't see the need to limit the user >> >> > > Why do you think that it limits the user? > > >>> 6. You seem to also invent another attribute module. Why can't you use >>> Class-XSAccessor , Moose or possibly even Class-Accessor? >>> >> [Dov L.] Please explain. I don't understand >> >> > > setters/getters/accessors are functions that are used to provide a more > robust > API that instead of doing: > > <<<< > $self->{'myfield'} = $value; > > > You do: > > <<<< > $self->myfield($value); > > > Or: > > <<<< > $self->set_myfield($value); > > > Or whatever. Likewise for getters or mutators. > > There are plenty of modules to do that on the CPAN, and from what I see > you've > implemented something of your own. See: > > http://www.shlomifish.org/lecture/Perl/Newbies/lecture5/accessors/ > > >>> 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). >>> >> [Dov L.] Why is regex injection bad? I use it for matching the files >> >> > > What if $file contains q{.*.*.*} and other crazy stuff that will take a lot > of > time to match and may not yield the right result. Generally, seeing a string > interpolated inside a regex is a red-flag that you want \Q ... \E , etc. See > also: > > http://perldoc.perl.org/functions/quotemeta.html > > Regards, > > Shlomi Fish > > >>> -------------------------- >>> >>> 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
