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

Levenglick Dov-RM07994 dov at freescale.com
Wed Oct 21 07:46:51 PDT 2009


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. 

 
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?

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
> Perl at perl.org.il
> http://mail.perl.org.il/mailman/listinfo/perl
> 

-- 
-----------------------------------------------------------------
Shlomi Fish       http://www.shlomifish.org/
What does "Zionism" mean? - http://shlom.in/def-zionism

Chuck Norris read the entire English Wikipedia in 24 hours. Twice.



More information about the Perl mailing list