Re: IMAP email client: style & security

2010-05-11 Thread Eitan Adler
> perl style is to just use boolean tests and not check for equality to
> some constants. and this means not using FALSE and TRUE constants. so
> you should drop this habit as you won't see it much in perl.
>

Alright - changed. This is exactly why I sent this email to the list - to
learn perl style. If you see anything else that is not often used in perl or
could be improved please let me know.
As I said in the OP this is my first perl program and I'm looking to improve
my style.

>
> uri
>
> --
> Uri Guttman  --  u...@stemsystems.com    http://www.sysarch.com--
> -  Perl Code Review , Architecture, Development, Training, Support
> --
> -  Gourmet Hot Cocoa Mix    http://bestfriendscocoa.com-
>


Re: IMAP email client: style & security

2010-05-11 Thread Uri Guttman
> "EA" == Eitan Adler  writes:

  >> Constants are usually written in all uppercase to distinguish them from
  >> keywords, functions, operators and subroutines.  How did you choose the
  >> arbitrary values 0 and 1 for false and true instead of using other values?
  >> Why did you name them false and true instead of zero and one?
  >> 

  EA> I changed them to uppercase. I call them TRUE and FALSE in order
  EA> to get the appearance of a Boolean construct.  In my opinion $var
  EA> = TRUE; is easier to read than $var = 1; I could have used any
  EA> arbitrary number that evaluates to true or false.

do you realize perl has 5 false values and rarely do you see constants
like TRUE??

these are all false:

0, 0.0, '0', '' and undef.

so defining a FALSE makes little sense if you could possibly get any of
those. also checking for a FALSE constant can be tricky. do you do $bool
== FALSE or eq FALSE? you need to know the type of FALSE for that.

perl style is to just use boolean tests and not check for equality to
some constants. and this means not using FALSE and TRUE constants. so
you should drop this habit as you won't see it much in perl. 

uri

-- 
Uri Guttman  --  u...@stemsystems.com    http://www.sysarch.com --
-  Perl Code Review , Architecture, Development, Training, Support --
-  Gourmet Hot Cocoa Mix    http://bestfriendscocoa.com -

-- 
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/




Re: IMAP email client: style & security

2010-05-11 Thread Eitan Adler
 I made the changes below and I'd like to know if there is anything else I
could do to improve the quality of my code.

use constant false => 0;
>>use constant true  => 1;
>>
>
> Constants are usually written in all uppercase to distinguish them from
> keywords, functions, operators and subroutines.  How did you choose the
> arbitrary values 0 and 1 for false and true instead of using other values?
>  Why did you name them false and true instead of zero and one?
>
I changed them to uppercase. I call them TRUE and FALSE in order to get the
appearance of a Boolean construct.
In my opinion $var = TRUE; is easier to read than $var = 1; I could have
used any arbitrary number that evaluates to true or false.

But why do you need to define those keys at all when autovivification can
> usually take care it for you?
>
a) readability (it tells you right away what possible keys I will use)
b) I didn't know about autovivification


>
>
> You don't have a default {} block so you are saying that $coml[0] will only
> ever contain 'command' or 'reply'?  The only thing you do with $command{
> reply } is set it to 0 at the beginning and set it to 1 here so why do you
> even have this variable?
>

Its for an unfinished feature in which is it would either email the output
back to me or save it as a file.

Thanks for your reply and time.

John
> --
> The programmer is fighting against the two most
> destructive forces in the universe: entropy and
> human stupidity.   -- Damian Conway
>
> --
> To unsubscribe, e-mail: beginners-unsubscr...@perl.org
> For additional commands, e-mail: beginners-h...@perl.org
> http://learn.perl.org/
>
>
>


Re: IMAP email client: style & security

2010-05-10 Thread John W. Krahn

Eitan Adler wrote:

I wrote a program to fetch email from a IMAP account, look to see if I sent
it, if yes execute any commands in the email and email the results back to
via a SMTP server.

1) This being my first perl program I'd like to know if I'm using the proper
perl idioms and language features. Is there anything I could do to improve
my code?


Sure.



use constant false => 0;
use constant true  => 1;


Constants are usually written in all uppercase to distinguish them from 
keywords, functions, operators and subroutines.  How did you choose the 
arbitrary values 0 and 1 for false and true instead of using other 
values?  Why did you name them false and true instead of zero and one?




sub doMessage {
print "Doing a message...\n";
my %command = ();
$command{"command"} = undef;
$command{"reply"} = false;


Creating and initializing a hash is usually done like:

my %command = (
command => undef,
reply   => false,
);

But why do you need to define those keys at all when autovivification 
can usually take care it for you?




my @lines = split('\n',$_[0]);
my $m_id = $_[1];
foreach(@lines)
{
my @coml = split(":",$_,2);
print "Command: ". $coml[0];
print "\nText:" .$coml[1];
print "\n";
given(uc $coml[0])
{
when("COMMAND")
{
$command{"command"} = $coml[1];
print PUSHCOLOR BLUE . $coml[1] . POPCOLOR 
."\n";
}
when ("REPLY")
{
$command{"reply"} = true;
}


You don't have a default {} block so you are saying that $coml[0] will 
only ever contain 'command' or 'reply'?  The only thing you do with 
$command{ reply } is set it to 0 at the beginning and set it to 1 here 
so why do you even have this variable?




}
}
my $res;
if (defined($command{"command"}))
{
$res = doCommand($command{"command"});
}
sendResults($res, $m_id);
print "\n";
}



Perhaps you only need something like this:

sub doMessage {
print "Doing a message...\n";

my ( $msg, $m_id ) = @_;

if ( $msg =~ /(?s:.*)^command:(.+)/im ) {
print PUSHCOLOR BLUE, $1, POPCOLOR, "\n";
sendResults( doCommand( $1 ), $m_id );
}

print "\n";
}




John
--
The programmer is fighting against the two most
destructive forces in the universe: entropy and
human stupidity.   -- Damian Conway

--
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/