[thelist] Perl: Writing to file not working......

[Matthew Brooks]

CDitty <mail at redhotsweeps.com> wrote:
>
>Can someone take a look at this and see what is wrong?

This jumped out at me:

>	foreach $emailfile(@emailfile) { # Loops through for each record
>	($new_email) = split(/\|/, $logemailfile);

Hmmm. Where did the variable $logemailfile suddenly appear from? It doesn't
match anything specified in the foreach statement. Try this instead:

	foreach (@emailfile) { # Loops through for each record
	$new_email = split(/\|/, $_);

You don't even need the variable name $emailfile; instead, $_ grabs each
item from the array as it loops.

Also, for efficiency, move this line:

>	$email = lc($email);         # lowercase

outside (i.e. before) the foreach statement; you only need to do it once,
not on every iteration.

>	if('$new_email' eq '$email'){
>		last;
>	}
>	else{
>		$ok_to_log_email='1';
>	}

The ' quotes around the two variable names are turning them into literal
strings, so every time the script arrives here it's going to compare the
text string '$new_email' with the text string '$email' - which will
*always* be false. Ditch the quotes, thus:

	if($new_email eq $email) {

Likewise, numbers don't need quotes:

>	if($ok_to_log_email == '1'){

So try this instead:

	if($ok_to_log_email == 1) {

A thought: are you at any point checking the email address ($email) to see
whether it is valid? If not, this script may add a non-valid address or
even a nonsense string to the file. To avoid that, and possibly to avoid
security issues too, you might want to think about checking $email and
rejecting invalid types. I use this string to match for an *invalid* address:

if ($email !~ /^\w{1}[\w-.]*\@[\w-.]+\.{1}[\w-.]+$/)

It seems to work fine, but I could have missed something; everyone seems to
do this a different way. Do a Google search and you'll see what other
people have to say about this.

Also, for security, use strict, and flag your shebang (the opening #!
statement) with -Tw. You'll have to predeclare all your variables and
doubtless change some other things, but you'll sleep easier knowing it's
secure.

>#Opens the counter file to get current number of plays

This doesn't really describe what the script is doing, although that won't
affect how the script runs, of course.

This should be enough to be going on with. If it still doesn't work, come
back here and we'll take another look.

HTH  ...  Matt