Illegal character in prototype for main

Discussion in 'Perl Misc' started by hillgoogle@charter.net, Aug 23, 2011.

  1. Guest

    I have an issue in my perl and its due to my mis-underatanding of perl
    sub.

    In my main pgm i have this statement that is calling my mail program.
    It works fine UNLESS i have a user like:
    tim.o'. When my user is just like
    then its fine. The error is "Illegal
    character in prototype for main::SendMail : $,$,$ "

    &SendMail($eml,$mail_text,$mail_sub);

    sub SendMail($,$,$)
    {
    #who the email is being sent to
    my $To = shift;

    #the message body
    my $mymsg = shift;

    #the mail subject
    my $mysubject = shift;

    #From email address
    my $from = "me\@somecompany.com";

    my $msg = new MIME::Lite
    To =>$To,
    From =>$from,
    Subject =>$mysubject,
    Type =>'text/html',
    Data =>$mymsg;

    $msg->send;
    }

    Should I be putting these 3 vars in an array and then using that
    array?

    Thanks for your help. Mike
     
    , Aug 23, 2011
    #1
    1. Advertising

  2. Uri Guttman Guest

    >>>>> "h" == hillgoogle <> writes:


    h> In my main pgm i have this statement that is calling my mail program.
    h> It works fine UNLESS i have a user like:
    h> tim.o'. When my user is just like
    h> then its fine. The error is "Illegal
    h> character in prototype for main::SendMail : $,$,$ "

    h> &SendMail($eml,$mail_text,$mail_sub);

    don't use & for calling subs. actually it will also bypass the prototype
    you are using

    h> sub SendMail($,$,$)

    and speaking of prototypes, they are not a good thing in general and
    rarely needed. the , is not a prototype char and that is the bug you are
    seeing. it should just be $$$. but since you bypass it by calling with &
    you can see it is of little use here. just drop using them unless you
    find a reason to use them.
    h> {
    h> #who the email is being sent to

    useless comment.

    h> my $To = shift;

    h> #the message body
    h> my $mymsg = shift;

    h> #the mail subject
    h> my $mysubject = shift;

    much better to just assign to a list of vars:

    my( $to, $msg, $subject ) = @_ ;

    h> #From email address
    h> my $from = "me\@somecompany.com";

    use single quotes for literals without interpolation. and then you don't
    need the \@

    my $from = '';


    h> my $msg = new MIME::Lite

    don't use indirect object calls. perldoc perlobj explains why. use a
    direct call like this:

    MIME::Lite->new(

    uri

    --
    Uri Guttman -- uri AT perlhunter DOT com --- http://www.perlhunter.com --
    ------------ Perl Developer Recruiting and Placement Services -------------
    ----- Perl Code Review, Architecture, Development, Training, Support -------
     
    Uri Guttman, Aug 23, 2011
    #2
    1. Advertising

  3. Guest

    On Aug 23, 4:21 pm, "Uri Guttman" <> wrote:
    > >>>>> "h" == hillgoogle  <> writes:

    >
    >   h> In my main pgm i have this statement that is calling my mail program.
    >   h> It works fine UNLESS i have a user like:
    >   h> tim.o'. When my user is just like
    >   h> then its fine. The error is "Illegal
    >   h> character in prototype for main::SendMail : $,$,$ "
    >
    >   h> &SendMail($eml,$mail_text,$mail_sub);
    >
    > don't use & for calling subs. actually it will also bypass the prototype
    > you are using
    >
    >   h> sub SendMail($,$,$)
    >
    > and speaking of prototypes, they are not a good thing in general and
    > rarely needed. the , is not a prototype char and that is the bug you are
    > seeing. it should just be $$$. but since you bypass it by calling with &
    > you can see it is of little use here. just drop using them unless you
    > find a reason to use them.
    >   h> {
    >   h> #who the email is being sent to
    >
    > useless comment.
    >
    >   h> my $To = shift;
    >
    >   h> #the message body
    >   h> my $mymsg = shift;
    >
    >   h> #the mail subject
    >   h> my $mysubject = shift;
    >
    > much better to just assign to a list of vars:
    >
    >         my( $to, $msg, $subject ) = @_ ;
    >
    >   h> #From email address
    >   h> my $from = "me\@somecompany.com";
    >
    > use single quotes for literals without interpolation. and then you don't
    > need the \@
    >
    >         my $from = '';
    >
    >   h> my $msg = new MIME::Lite
    >
    > don't use indirect object calls. perldoc perlobj explains why. use a
    > direct call like this:
    >
    >         MIME::Lite->new(
    >
    > uri
    >
    > --
    > Uri Guttman  --  uri AT perlhunter DOT com  ---  http://www.perlhunter.com--
    > ------------  Perl Developer Recruiting and Placement Services  -------------
    > -----  Perl Code Review, Architecture, Development, Training, Support -------


    Uri, so you are saying my code shold be as follows:
    ******************************************************************************

    SendMail($eml,$mail_text,$mail_sub);

    sub SendMail($$$)
    {
    my ($To, $mymsg, $mysubject) = @_;

    my $from = "me\@somecompany.com";

    my $msg = MIME::Lite->new(
    To =>$To,
    From =>$from,
    Subject =>$mysubject,
    Type =>'text/html',
    Data =>$mymsg);

    $msg->send;
    }

    ******************************************************************************
    Right?
    :) Mike
     
    , Aug 23, 2011
    #3
  4. "Uri Guttman" <> writes:
    >>>>>> "h" == hillgoogle <> writes:


    [...]

    > h> &SendMail($eml,$mail_text,$mail_sub);
    >
    > don't use & for calling subs.


    At least not when calling them with arguments. &something is useful in
    subroutines performing some 'auxiliary task' and then 'forwarding' the
    call to another routine: It means 'invoke something and use the
    current @_ as argument list'. Semantically, this is (mostly)
    equivalent to something(@_) except that it is shorter and the overhead
    of creating a new @_ whose content is identical to the present @_ is
    avoided.
     
    Rainer Weikusat, Aug 23, 2011
    #4
  5. Guest

    On Aug 23, 4:41 pm, Rainer Weikusat <> wrote:
    > "Uri Guttman" <> writes:
    > >>>>>> "h" == hillgoogle  <> writes:

    >
    > [...]
    >
    > >   h> &SendMail($eml,$mail_text,$mail_sub);

    >
    > > don't use & for calling subs.

    >
    > At least not when calling them with arguments. &something is useful in
    > subroutines performing some 'auxiliary task' and then 'forwarding' the
    > call to another routine: It means 'invoke something and use the
    > current @_ as argument list'. Semantically, this is (mostly)
    > equivalent to something(@_) except that it is shorter and the overhead
    > of creating a new @_ whose content is identical to the present @_ is
    > avoided.


    OK ... then it should be:

    ******************************************************************
    &SendMail($eml,$mail_text,$mail_sub);

    sub SendMail($$$)
    {
    my ($To, $mymsg, $mysubject) = @_;

    my $from = '';

    my $msg = MIME::Lite->new(
    To =>$To,
    From =>$from,
    Subject =>$mysubject,
    Type =>'text/html',
    Data =>$mymsg);

    $msg->send;
    }
    ******************************************************************

    Better? TY Mike
     
    , Aug 23, 2011
    #5
  6. Uri Guttman Guest

    >>>>> "h" == hillgoogle <> writes:

    h> Uri, so you are saying my code shold be as follows:

    h> SendMail($eml,$mail_text,$mail_sub);

    h> sub SendMail($$$)

    drop the prototype.

    sub SendMail {

    and most put the opening { on the sub line.

    h> {
    h> my ($To, $mymsg, $mysubject) = @_;

    don't mix upper/lower case names. also why the my prefix? they are
    scoped with my so $msg and $subject are better names.

    also indent code in the sub body.

    h> my $from = "me\@somecompany.com";

    you didn't change the quotes as i showed you.

    h> my $msg = MIME::Lite->new(

    h> To =>$To,
    h> From =>$from,
    h> Subject =>$mysubject,
    h> Type =>'text/html',
    h> Data =>$mymsg);

    some white space helps there. also don't put the close ); on the data
    line. it is hard to see and means you can't cut/paste that line or move
    the close ) as easily.

    h> $msg->send;
    h> }

    h> Right?

    not completely but better. this is how i would do it. note ALL the
    little changes like whitespace and name fixes.

    send_mail( $eml, $mail_text, $mail_sub );

    sub send_mail {

    my( $to, $msg, $subject ) = @_;

    my $from = '';

    my $msg = MIME::Lite->new(
    To => $to,
    From => $from,
    Subject => $subject,
    Type => 'text/html',
    Data => $msg
    );

    $msg->send() ;
    }

    uri

    --
    Uri Guttman -- uri AT perlhunter DOT com --- http://www.perlhunter.com --
    ------------ Perl Developer Recruiting and Placement Services -------------
    ----- Perl Code Review, Architecture, Development, Training, Support -------
     
    Uri Guttman, Aug 23, 2011
    #6
  7. Uri Guttman Guest

    >>>>> "h" == hillgoogle <> writes:

    h> On Aug 23, 4:41 pm, Rainer Weikusat <> wrote:
    >> "Uri Guttman" <> writes:
    >> >>>>>> "h" == hillgoogle  <> writes:

    >>
    >> [...]
    >>
    >> >   h> &SendMail($eml,$mail_text,$mail_sub);

    >>
    >> > don't use & for calling subs.

    >>
    >> At least not when calling them with arguments. &something is useful in
    >> subroutines performing some 'auxiliary task' and then 'forwarding' the
    >> call to another routine: It means 'invoke something and use the
    >> current @_ as argument list'. Semantically, this is (mostly)
    >> equivalent to something(@_) except that it is shorter and the overhead
    >> of creating a new @_ whose content is identical to the present @_ is
    >> avoided.


    h> OK ... then it should be:

    h> ******************************************************************
    h> &SendMail($eml,$mail_text,$mail_sub);

    no.

    h> sub SendMail($$$)

    and no again.

    drop both the & and the prototype. one disables the other and the
    prototype is useless to you here. it is not a good feature in general
    and has only a few very narrow uses.

    uri

    --
    Uri Guttman -- uri AT perlhunter DOT com --- http://www.perlhunter.com --
    ------------ Perl Developer Recruiting and Placement Services -------------
    ----- Perl Code Review, Architecture, Development, Training, Support -------
     
    Uri Guttman, Aug 23, 2011
    #7
  8. Am 23.08.2011 23:46, schrieb :
    > On Aug 23, 4:41 pm, Rainer Weikusat <> wrote:
    >> "Uri Guttman" <> writes:
    >>>>>>>> "h" == hillgoogle <> writes:

    >>
    >> [...]
    >>
    >>> h> &SendMail($eml,$mail_text,$mail_sub);

    >>
    >>> don't use & for calling subs.

    >>
    >> At least not when calling them with arguments. &something is useful in
    >> subroutines performing some 'auxiliary task' and then 'forwarding' the
    >> call to another routine: It means 'invoke something and use the
    >> current @_ as argument list'. Semantically, this is (mostly)
    >> equivalent to something(@_) except that it is shorter and the overhead
    >> of creating a new @_ whose content is identical to the present @_ is
    >> avoided.

    >
    > OK ... then it should be:
    >
    > ******************************************************************
    > &SendMail($eml,$mail_text,$mail_sub);
    >
    > sub SendMail($$$)


    No! Re-read what has been written. Using the & overrides possible
    prototypes - and in addition (and that's what Rainer's comment was
    about) it passes all current arguments to the sub if you don't use
    parameters.

    Maybe an example can explain this better:

    sub foo {
    print "Foo: @_";
    }
    sub bar { &foo }
    bar("Hello", 42);

    Here you're calling bar with two parameters. bar then calls foo. Since &
    has a special meaning, the program will output Foo: Hello 42. Try
    removing the & here!

    To summarise: don't use & if you don't know what it is for.


    Please use this:

    SendMail($eml,$mail_text,$mail_sub);

    sub SendMail {
    ...
    }
     
    Wolf Behrenhoff, Aug 24, 2011
    #8
  9. >>>>> "u" == Uri Guttman <> writes:

    u> drop the prototype.

    u> sub SendMail {

    u> and most put the opening { on the sub line.

    Don't make me get my torch and pitchfork.

    Charlton


    --
    Charlton Wilbur
     
    Charlton Wilbur, Aug 24, 2011
    #9
  10. Ted Zlatanov Guest

    On Tue, 23 Aug 2011 17:55:19 -0400 "Uri Guttman" <> wrote:

    UG> not completely but better. this is how i would do it. note ALL the
    UG> little changes like whitespace and name fixes.

    send_mail( $eml, $mail_text, $mail_sub );

    sub send_mail {

    my( $to, $msg, $subject ) = @_;

    my $from = '';

    my $msg = MIME::Lite->new(
    To => $to,
    From => $from,
    Subject => $subject,
    Type => 'text/html',
    Data => $msg
    );

    $msg->send() ;
    }

    (end Uri's example)

    I wanted to suggest something perhaps more generally useful, where all
    the parameters you pass to your function will simply become hash key
    overrides. Also note the warning at the end.

    Ted

    #+begin_src perl

    #!/usr/bin/perl

    use warnings;
    use strict;
    use Data::Dumper;
    use MIME::Lite;

    send_mail( From => '', To => "you", Data => "message data", Subject => "subject you want" );

    sub send_mail {

    my %settings = (
    To => "Default Recipient",
    From => "Default Sender",
    Subject => "Default Subject",
    Type => 'text/html',
    Data => "Default Message Text"
    );

    my %overrides = @_;

    $settings{$_} = $overrides{$_} foreach sort keys %overrides;

    my $msg = MIME::Lite->new(%settings);
    $msg->send() if defined $msg;
    warn "Could not use settings to create message " . Dumper(\%settings)
    unless defined $msg;
    }

    #+end_src
     
    Ted Zlatanov, Aug 24, 2011
    #10
  11. Ted Zlatanov Guest

    On Wed, 24 Aug 2011 13:04:17 -0500 Tad McClellan <> wrote:

    TM> I often start out my subroutines thusly:

    TM> sub send_mail {
    TM> my %args = (
    TM> To => "Default Recipient",
    TM> From => "Default Sender",
    TM> Subject => "Default Subject",
    TM> Type => 'text/html',
    TM> Data => "Default Message Text",
    TM> @_
    TM> );

    TM> ...

    TM> There is no need to explicitly hashify and overwrite, I just let the
    TM> usual "last one wins" behavior of a list assignment to a hash
    TM> handle all of that for me.

    I thought that might feel a little too much like black magic to the OP,
    who is probably a Perl beginner. Also it's easier to hook further
    changes or messages based on certain keys if you do the overrides
    explicitly. FWIW I would probably do the list assignment in my own
    code but I tend to overthink examples I post to c.l.p.misc :)

    Ted
     
    Ted Zlatanov, Aug 25, 2011
    #11
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. William Brogden
    Replies:
    1
    Views:
    8,390
    Manoj S. P.
    Jun 30, 2003
  2. June Lee
    Replies:
    2
    Views:
    819
    Jim Cobban
    Apr 13, 2008
  3. Replies:
    9
    Views:
    213
    Thomas 'PointedEars' Lahn
    May 26, 2006
  4. Replies:
    3
    Views:
    278
  5. javascript fish
    Replies:
    0
    Views:
    181
    javascript fish
    Oct 11, 2008
Loading...

Share This Page