[Israel.pm] Fwd: TelFOSS Soliciting aTalk aboutAutomated SoftwareTesting

Levenglick Dov-RM07994 RM07994 at freescale.com
Sun Oct 3 02:30:27 PDT 2010


Thanks, I'll rectify

 
Best Regards,
Dov Levenglick
SmartDSP OS Development Leader
-----Original Message-----
From: Shlomi Fish [mailto:shlomif at iglu.org.il] 
Sent: Wednesday, September 22, 2010 14:51
To: perl at perl.org.il
Cc: Levenglick Dov-RM07994
Subject: Re: [Israel.pm] Fwd: TelFOSS Soliciting aTalk aboutAutomated
SoftwareTesting

On Sunday 19 September 2010 19:43:46 Shlomi Fish wrote:
> On Sunday 19 September 2010 16:10:53 Levenglick Dov-RM07994 wrote:
> >
https://docs.google.com/leaf?id=0B5HrAp2ZF5LqMDk3NmRlMTUtYmUxZS00ODg4LTh
> > hZGUtY2MwNzI4ZmYwMmY0&hl=en&authkey=CKfbgaYI
> 
> Your mailer broke the URL into two. Here is the unbroken URL:
> 
>
https://docs.google.com/leaf?id=0B5HrAp2ZF5LqMDk3NmRlMTUtYmUxZS00ODg4LTh
hZG
> UtY2MwNzI4ZmYwMmY0&hl=en&authkey=CKfbgaYI
> 
> And here is a shortened URL:
> 
> http://xrl.us/bhzrfj
> 
> I initially believed it was a Google Document (at which point I wanted
to
> ask if you had any embedded POD), but now that I've downloaded it, I
see
> that it's a .zip file containing a few .pm files. I'll take a look at
it
> later.


> 
> Just one note:
> 
> {{{
> use lib 'STP';
> }}}
> 
> This is not needed as "use STP::Array;" will already look under
> STP/Array.pm.
> 

OK, here are some other notes:

1. Your POD reads: "Each STP object" - please continue it.

2. 

{{{
require Exporter;

our (@ISA, @EXPORT, @EXPORT_OK, $VERSION, $DEBUG) ;

@ISA        = qw(Exporter);
}}}

This is better written as:

{{{
use parent 'Exporter';

our (@EXPORT, @EXPORT_OK, $VERSION, $DEBUG) ;
}}}

Or perhaps:

{{{
use base 'Exporter';

our (@EXPORT, @EXPORT_OK, $VERSION, $DEBUG) ;
}}}


3. 

[code]
@EXPORT     = qw(new add status ok passed find);
[/code]

Why are you exporting "new"? You shouldn't export the constructor. Also,

aren't add() and friends methods which should be used as part of the
class? If 
so, they shouldn't be exported, and you can get rid of inheriting from 
Exporter.pm.

4. The lines in your POD in STP.pm are too long. Please keep them at 80 
characters or 78 characters or so.

5. The synopsis for ->new reads:

    my $test = STP->new('name'=>'criteria_name', values=>[
{'value1'=>'val1' 
}, {'value2'=>'val2' } ]);

But it does not appear to match the constructor.

6.

{{{
my $class = ref($this) || $this;
}}}

Please don't do a $my_obj->new() :

http://www.stonehenge.com/merlyn/UnixReview/col52.html

7. The signature of add seems wrong - you don't appear to be accepting a
hash 
as method arguments. Furthermore in the POD:

{{{
$test->add[\%criteria, %more_criteria];
}}}

This is invalid syntax.

8.

{{{ 
carp "You are creating an element that it already created!".
}}}

This is invalid English.

9. 

{{{
$this->[(TEST_LOC)]->{$hashref->{'name'}} = (ref $hashref->{'val'} eq
'ARRAY') 
?
            STP::Array->new($hashref) : STP::Scalar->new($hashref);
}}}

Maybe just require to pass STP::Array and STP::Scalar directly?

10. I also see you're using an array ref for the object with TEST_LOC as
a 
slot name like this:

$this->[(TEST_LOC)]

Why not just use an accessor? See:

http://www.shlomifish.org/lecture/Perl/Newbies/lecture5/accessors/

10.

Some of the methods in STP.pm lack POD. Maybe try using POD::Coverage.

11. 

{{{
 map {return (STP_FAIL) if $_->status() != (STP_OK)} (values %{ $this-
>[(TEST_LOC)] });
}}}

Just use a foreach here. Or maybe use List::MoreUtils' any(), all(),
notall() 
or none().

12. 

[code]
map {$ok = $_->analyze($verbose, $no_ret_on_err); return $ok if $ok != 
(STP_OK) and not $no_ret_on_err} (values %{ $this->[(TEST_LOC)] });
[/code]

Again, don't return from within a map.

13. 

[code]
sub find
{
    my ($this, $name) = @_;
    return (exists $this->[(TEST_LOC)]->{name}) ?
$this->[(TEST_LOC)]->{name} 
: undef;
}
[/code]

find is a bad name for this method. Use lookup_$something or something.

-------------------------

I just covered STP.pm - I'll go over the other modules once my comments
are 
fixed and a new version is uploaded.

Happy Sukkoth!

Regards,

	Shlomi Fish

-- 
-----------------------------------------------------------------
Shlomi Fish       http://www.shlomifish.org/
Why I Love Perl - http://shlom.in/joy-of-perl

<rindolf> She's a hot chick. But she smokes.
<go|dfish> She can smoke as long as she's smokin'.

Please reply to list if it's a mailing list post - http://shlom.in/reply
.




More information about the Perl mailing list