[Israel.pm] File::Find::Recursive

Uri Bruck uri at bruck.co.il
Thu Oct 22 00:37:28 PDT 2009


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:shlomif at iglu.org.il]
>> Sent: Wednesday, October 21, 2009 20:05
>> To: perl at perl.org.il
>> 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:shlomif at iglu.org.il]
>>> Sent: Wednesday, October 21, 2009 15:30
>>> To: perl at perl.org.il
>>> 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
>>>> Perl at perl.org.il
>>>> http://mail.perl.org.il/mailman/listinfo/perl
>>>>         
>
>   



More information about the Perl mailing list