By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
438,179 Members | 970 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 438,179 IT Pros & Developers. It's quick & easy.

Problem Passing data between subroutines

Kelicula
Expert 100+
P: 176
Hey everyone!
I am back.
I have a script that handles the private messaging for a site I am working on and I created a subroutine to handle the sending of a message.

I want to check if the "to" field was left blank, contains a user name that doesn't exist or has invalid characters in it.

Everything worked fine, until I integrated the sending of messages to multiple recipients. What I did was create another subroutine to "prepare" the sending. ie: loop through the different user names, and extract proceeding, or trailing whitespace, and them send it to the "send message" subroutine.

Now for some reason it cannot tell if the user left the "to" field blank.
No matter what perl considers the "$recepient" to be true. I've tried:

Expand|Select|Wrap|Line Numbers
  1.  
  2. if(!$recepient){
  3. do something..
  4. }
  5.  
  6. # AND
  7.  
  8. if($recepient !~ /[a-zA-Z]/g){
  9. really bad but was just testing..
  10. }
  11.  
  12. # AND 
  13. if(length $recepient <1){
  14. do something
  15. }
  16.  
Nothing works...
If I open a message and leave the "to" field blank, and click send, I just get the "Message Sent!" message...


I can't understand it. I get the appropriate error message for both of the other "if" checks...

Here is the code.
Expand|Select|Wrap|Line Numbers
  1. my $error = qq~<font color="red"><small><ul>~; 
  2.  
  3. sub sendMessage {
  4.  
  5. my ($recepient, $title, $body) = @_;
  6.  
  7. $sth = $dbh->prepare(qq~SELECT id, online, email, newsletter FROM users WHERE username=?~);
  8. $sth->execute($recepient)|| Tools->listErr( $sth->errstr );
  9. my @rece = $sth->fetchrow_array;
  10. $sth->finish;
  11.  
  12. if(!$rece[0] && $recepient){
  13. $error .= qq~<li>That user: $recepient, does not exist. Make sure you punctuated correctly</li>~;
  14. }
  15.  
  16. if($recepient eq '' || $recepient eq undef){
  17. $error .= qq~<li>You left the "To" field blank!</li>~;
  18. }
  19.  
  20. if($recepient =~ /[^a-zA-Z0-9_\-\s]/){
  21. $error .= qq~<li>You have invalid characters in the "To" field.<br /> You may only use a-z, A-Z, 0-9, spaces,<br /> underscores and dashes.</li>~;
  22. }
  23.  
  24. unless(length $error > 29){
  25.  
  26. $sth = $dbh->prepare(qq~ INSERT INTO pms (user, recepient, title, body) values(?, ?, ?, ?)~);
  27. $sth->execute($user->{id}, $rece[0], $title, $body)|| Tools->listErr( $sth->errstr );
  28.  
  29. if(($rece[1] == 0) && ($rece[3] == 1)){
  30. my $message = qq~You have a new "Private Message" from $user->{username}, waiting for you on \nLiveAudioMag!\n\nClick The link below to log-in and read your message\nhttp://www.liveaudiomag.com/cgi-bin/index.cgi\n\nDo not reply to this message, it was auto-generated and the replies are not read. \nTo unsubscribe to these notifications, Uncheck the "Keep me up to date on LiveAudioMag news, and features." checkbox in your profile edit page.~;
  31. Tools->sendmail('"LiveAudioMag" <webmaster@liveaudiomag.com>', "$rece[2]", 'New Private Message!', "$message")||
  32. Tools->listErr( $! );
  33.     }
  34.   }
  35.   return $error;
  36. }
  37.  
  38.  
  39.  
  40. sub prepSend {
  41.  
  42. my @recepient = split(/,/, $q->param('to'));
  43. my $title = $q->param('subject')|| "none";
  44. my $body = $q->escapeHTML( $q->param('body') );
  45.  
  46. for(@recepient){
  47.  
  48. $_ =~ s/^\s|\s$//g;
  49.  
  50. if(length &sendMessage($_, $title, $body) > 29){
  51. $error .= qq~<br />Click "Back" on your browser to retrieve message, and resolve error.</ul></small></font>~;
  52.  
  53. Tools->create2( 'compose', 'pm', {
  54.     user    => $user,
  55.         cgi    => $q,
  56.         error    => $error,
  57.         contacts => $contacts
  58.         }) || Tools->listErr( $Tools::t->error );
  59.     exit(0);
  60.         }
  61. }
  62.  
  63. $error = qq~<font color="blue">Message Sent!</font>~;
  64. &openInbox($error);
  65. }
It's just a snippet, but I'm SURE the problem lies within this code.

When someone sends a message the "prepSend" sub is called.

Before I broke up the work, and had one sub to handle it all (only taking one user name) it worked fine.
The error WILL work if the user name is not in the database, or contains invalid characters, but it will not seem to notice that the "to" field is blank.

Is this something to do with arrays?
I was under the impression that null, undef, 0, '' are all the same thing to perl; false.

Anyone see my mistake? (Other than the misspelling of recipient..hehehe)
Thanks in advance...
Jun 26 '08 #1
Share this Question
Share on Google+
11 Replies


KevinADC
Expert 2.5K+
P: 4,059
At the end of sub prepSend you have:

Expand|Select|Wrap|Line Numbers
  1. $error = qq~<font color="blue">Message Sent!</font>~;
  2. &openInbox($error);
it looks like that will always be evaluated unless the exit() function is called before that point in the code. And since $error is defined as above, that is always the message it will contain and send to sub openInbox.
Jun 26 '08 #2

KevinADC
Expert 2.5K+
P: 4,059
I suggest you add some print commands to watch what the script is doing while it runs if you haven't done that already.
Jun 26 '08 #3

Kelicula
Expert 100+
P: 176
Yes, I have the sendMessage routine return the error, so that I can see it's length.
The error message begins with 29 characters if an error occurs it gets perpended to, thus lengthening the string. And I do "exit(0)" after that. See lines 50 through 60.

The error checking works for invalid characters or a user name that doesn't exist.
It just seems that by taking a part of an array, and passing it, there is always a "true" value. Even if it's blank. Maybe it is recognizing the reference to a memory allocation?

But I am not using a reference?

Can't figure it. I have tried printing the value of $_ and it is blank (to the naked eye)
Jun 27 '08 #4

KevinADC
Expert 2.5K+
P: 4,059
Something else is going on because this code seems to work, which is an attempt to simulate your code with some dummy data:

Expand|Select|Wrap|Line Numbers
  1. use strict;
  2. use warnings;
  3.  
  4. my $to = 'fee,fi, foe ,  , fum';
  5. my @recipient = split(/,/,$to);
  6.  
  7. for (@recipient) {
  8.    $_ =~ s/^\s|\s$//g;
  9.    print "[$_] ", checkIt($_), "\n";
  10. }
  11.  
  12. sub checkIt {
  13. my $error;
  14. my ($recepient) = @_;
  15. my @rece = ();# to simulate a blank $rece[0]
  16.  
  17. if(!$rece[0] && $recepient){ # this one should always be true since there is no $rece[0]
  18.    $error .= qq~<li>That user: $recepient, does not exist. Make sure you punctuated correctly</li>~;
  19. }
  20.  
  21. if($recepient eq '' || $recepient eq undef){
  22.    $error .= qq~<li>You left the "To" field blank!</li>~;
  23. }
  24.  
  25. if($recepient =~ /[^a-zA-Z0-9_\-\s]/){
  26.    $error .= qq~<li>You have invalid characters in the "To" field.~;
  27.  
  28. }
  29.  
  30. return $error;
  31.  
output:

Expand|Select|Wrap|Line Numbers
  1. [fee] <li>That user: fee, does not exist. Make sure you punctuated correctly</li>
  2. [fi] <li>That user: fi, does not exist. Make sure you punctuated correctly</li>
  3. [foe] <li>That user: foe, does not exist. Make sure you punctuated correctly</li>
  4. [] <li>You left the "To" field blank!</li>
  5. [fum] <li>That user: fum, does not exist. Make sure you punctuated correctly</li>
Jun 27 '08 #5

KevinADC
Expert 2.5K+
P: 4,059
Try changing this:

Expand|Select|Wrap|Line Numbers
  1. $_ =~ s/^\s|\s$//g;
to:

Expand|Select|Wrap|Line Numbers
  1. $_ =~ s/^\s*|\s*$//g; 
although I don't think that is the source of your problem.
Jun 27 '08 #6

eWish
Expert 100+
P: 971
To me this logic does not look correct. I am interpreting this as if $rece[0] is undef and $recipient is defined, print error.

Expand|Select|Wrap|Line Numbers
  1. if(!$rece[0] && $recipient){
  2.  
  3. $error .= qq~<li>That user: $recipient, does not exist. Make sure you punctuated correctly</li>~;
  4.  
  5. }
Are you wanting to compare them? If so, the this still will fail because $rece[0] looks like it is holding a numeric value and not an alphanumeric value like $recipient appears to contain.

I hope that I am not completely off here....

--Kevin
Jun 27 '08 #7

Kelicula
Expert 100+
P: 176
To me this logic does not look correct. I am interpreting this as if $rece[0] is undef and $recipient is defined, print error.

Expand|Select|Wrap|Line Numbers
  1. if(!$rece[0] && $recipient){
  2.  
  3. $error .= qq~<li>That user: $recipient, does not exist. Make sure you punctuated correctly</li>~;
  4.  
  5. }
Are you wanting to compare them? If so, the this still will fail because $rece[0] looks like it is holding a numeric value and not an alphanumeric value like $recipient appears to contain.

I hope that I am not completely off here....

--Kevin

No you have it right. @rec represents a data set from a database query. $recepient is the value entered by the user. So I am saying; if there is no result set from the database, but they DID enter something in the input, print error.

That is not the check that I'm having a problem from, because if I enter a username that does not exist I get the appropriate error. Which means that
lines 50 through 60 work fine.

Also if I enter a user name with invalid characters in it I get that error message too.

The only thing that doesn't work is the:
Expand|Select|Wrap|Line Numbers
  1.  
  2. if($recepient eq '' || $recepient eq undef){
  3. $error .= qq~<li>You left the "To" field blank!</li>~;
  4. }
  5.  
Test.

Scratching my head...
Jun 27 '08 #8

KevinADC
Expert 2.5K+
P: 4,059
Like I said, something else is going on because in my example there is a blank entry and it gets caught. See line 4 in the output.
Jun 27 '08 #9

KevinADC
Expert 2.5K+
P: 4,059
OK, I think I see the problem. If the "to" field is blank, the array is never populated with any data here:

Expand|Select|Wrap|Line Numbers
  1. my @recepient = split(/,/, $q->param('to'));
So the subsequent loop is never entered into:

Expand|Select|Wrap|Line Numbers
  1. for(@recepient){
None of the code inside the loop is ever evaluated if the array is empty. Thus the sub &sendMessage() is never called. No error is returned from the sub and your code executes the last lines in sub prepSend:


Expand|Select|Wrap|Line Numbers
  1. $error = qq~<font color="blue">Message Sent!</font>~;
  2. &openInbox($error);
Make sense?
Jun 27 '08 #10

Kelicula
Expert 100+
P: 176
OK, I think I see the problem. If the "to" field is blank, the array is never populated with any data here:

Expand|Select|Wrap|Line Numbers
  1. my @recepient = split(/,/, $q->param('to'));
So the subsequent loop is never entered into:

Expand|Select|Wrap|Line Numbers
  1. for(@recepient){
None of the code inside the loop is ever evaluated if the array is empty. Thus the sub &sendMessage() is never called. No error is returned from the sub and your code executes the last lines in sub prepSend:


Expand|Select|Wrap|Line Numbers
  1. $error = qq~<font color="blue">Message Sent!</font>~;
  2. &openInbox($error);
Make sense?
KevinADC you rock!
That is exactly the problem.
I changed it to check for the existence of @recepient and now it works.

Thank you!!
Jun 28 '08 #11

KevinADC
Expert 2.5K+
P: 4,059
You're welcome.
Jun 28 '08 #12

Post your reply

Sign in to post your reply or Sign up for a free account.