Re: Re: Re: problem with variable

Short answer: Your problem is here:

# Read in all the variables set by the form
CGI::ReadParse(*input);

$month = $input{'month'};
$date  =  $input{'date'};
$year  =  $input{'year'};
[download]

I don't know why not, but ReadParse is not working as you want it to in this instance. As a result $input{month} is undefined and so $month is too.

Since you're using CGI, and you know how to pull parameters out, I'd suggest you replace this block of code with something more like:

        my $month = $q->param('month');
        my $date  = $q->param('date');
        my $year  = $q->param('year');
        my $title = $q->param('title');
        my $message = $q->param('message');
[download]

This will fix the problem you've described (and make you aware of a lot more).

Long answer

Further comments about your code are below. Some are good, some are nitpicky.

```
#!/usr/local/bin/perl
use CGI qw/:all/;

my $q = new CGI;
[download]
```
There's no point in dragging in all of the CGI functions if you're going to create a CGI object. I'd suggest you change this to:
```
#!/usr/local/bin/perl -w
use strict;
use CGI;

my $q      = new CGI;
[download]
```
The use strict will help make the other monks here want to help you. Of course it'll help you write code that won't have some of the errors I'm going to cover now. The -w gives your warnings, and believe me, you want to notice them.
```
CGI::ReadParse(*input);
[download]
```
As mentioned above, this isn't working for you.
```
print "Content-type: text/html\n\n";
[download]
```
We're using CGI here. CGI's got a header function. We should use it here.
```
print $q->header();
[download]
```
That'll help when you want to start playing with cookies too.

if($action eq "add_data"){
        print "Your post was sucessful<br>";
        add_data();
}
else{
}

if($action eq "preview"){
        preview();
}
else{
}
[download]

If action can't be changed between the call to add_data and the check for preview then you might want to combine these:

if($action eq "add_data") {
        print "Your post was sucessful<br>";
        add_data($q, $numberoflines,$database);
} elsif ($action eq "preview") {
        preview($q, $numberoflines);
} else {
        print STDERR "Unrecognised action: [$action]\n";
}
[download]

```
# in CountCurrentRecords
open(DATABASE, "$database");
[download]
```
If you happen to be unlucky enough that the database file you'd like to check doesn't actually exist, or apache can't read it or whatever you're going to get some odd results here. Nothing good, certainly. (And if you don't have warnings turned on, you might not even notice!).

# in CountCurrentRecords
$number = 1;
# ...
$number = 0 if $number < 0;
[download]

That's not going to happen. So the line is useless.

```
# in preview
print "<html>
<HR>
  <table border='0' width='100%'>
...
[download]
```
I think the HTML standard expects double quotes (") around elements not single quotes "'". But this isn't a Perl issue.
```
# in add_data
$addfile = "events.txt";
[download]
```
We're using events.txt again. Perhaps we should define it once and pass it around so that we don't end up with a possibility of typoing it.
You're making a lot of global variables, such as $title, $date, and $month. You should consider explicitly passing variables to your functions rather than relying on what I call "call-by-magic". For example, you can rewrite your add_data subroutine to be complete as follows:
```
sub add_data {
        my ($q, $numberoflines, $addfile) = @_;

                # Assign intelligent defaults.
        my $month = $q->param('month') || 0;
        my $date  = $q->param('date') || 0;
        my $year  = $q->param('year') || 0;
        my $title = $q->param('title') || "";
        my $message = $q->param('message') || "";

        open(OUT, ">>$addfile") or die "Can't open $addfile\n";
        my $number  = $numberoflines;
        my $line = join '::', $number, $month, $date, $year, $title, $
+message;

        print OUT "$line\n";
        close OUT;
}
[download]
```
And call it like this:
```
if ($action eq "add_data") {
        print "Your post was sucessful<br>";
        add_data($q, $numberoflines, $database);
}
[download]
```
where $database has been set to "events.txt" at the top of your file. You'll note that I'm pulling $month, $date etc from the CGI object here. I do this in preview as well. As a result I don't pull these out in the greater scope because they are only used in these two functions. There isn't anything wrong with pulling them out and putting them into a hash at the top of your script, and then passing that hash around, but this was faster for me to type/cut-n-paste.

Hope this helps.

jarich

Comment on Re: Re: Re: problem with variable Select or Download Code