Sub routine problem

[jkmglasgow]

Thanks for all the replies, I'll use the strict and warning commands.
Basically I made a copy of the sub routine and changed the code to just print the 2 submit buttons, the original sub routine was being called in 4 different locations
so I changed one of the locations based on the logic to use the new copied sub-routine.
Also I realised that variable wasn't being recognized in the sub routine as I had defined it at a lower place in the control.

Thanks Everybody




Newbie to PERL/CGI - the people who wrote this are long gone, only
have o'reilly-Learing Perl and Elizabeth Castro perl & the CGI 2nd ed

I have altered a sub routine in a CGI script to add a submit button on data displayed in table format, see attachment.
This new button(CONTRACT) should only be displayed after the 1st 2 buttons(MODIFY and DELETE). However it appears after the 2nd set of submit
buttons also.
I have tried passing additional variables to the sub routine and they are always blank, there are 2 things that I don't understand(probably more, but don't know enough) Here is the code for the sub routine.

1) On line 238 it references a sub routine in a library what does this do, I have included this sub routine also.

2) On line 250 it references a variable $prtupd, it never passes this to the sub routine yet it contains the value it sets in the main module, when I
try to pass a variable by adding it to line 236 and in the line in the main module it is always empty(lines 247 and 248 confirms this for me)


230
231 sub PrintSal
232 {
233 ##print "<BR><BR></center>\n";
234 ##print "<P><font> COLOR=CC0000 size=2><B>Pcontr: <I> $prntcontracts</font><P>\n";
235 #Try not to use this thing any more -- I want to wax it but we use it in too many places to do that!
236 ($type,$record,@stuff) = @_;
237 $counter = 0;
238 print "<!--html-lib.pl::PrintRow-->\n";
239 foreach $cell (@stuff)
240 {
241 if($cell eq "") { $cell = "&nbsp\;"; }
242 print "<td bgcolor=\"$color\"><font face=\"Arial, Helvetica, sans-serif\" size=\"-1\">$cell</td>\n";
243 # print "<td bgcolor=\"$color\"><font face=\"Arial, Helvetica, sans-serif\" size=\"-1\">$stuff[$counter]</td>\n";
244 # $counter++;
245 }
246
247 print JKM "start of trail\n";
248 print JKM "$type, $record, CONTRACTs is $printcontracts, $prtupd\n";
249
250 if($prtupd == 1)
251 {
252 print<<END_OF_TWOBUTTONS;
253 <form method="post" action="/cgi-bin/mbm/ModifySetup.cgi"><input type="hidden" name="type" value="$type">
254 <input type="hidden" name="recordid" value="$record">
255 <td bgcolor="$color"><input type="submit" value="Modify"></td>
256 </FORM>
257 <form method="post" action="/cgi-bin/mbm/DeleteConfirm.cgi"><input type="hidden" name="type" value="$type">
258 <input type="hidden" name="recordid" value="$record">
259 <td bgcolor="$color"><input type="submit" value="Delete"></td>
260 </FORM>
261 END_OF_TWOBUTTONS
262 }
263 else
264 {
265 print "<td bgcolor=$color></td>";
266 print "<td bgcolor=$color></td>";
267 }
268
269 if (($prntcontracts == "YES") && ($prtupd == 1))
270 {
271 print<<END_OF_THIRD;
272 <form method="post" action="/ContractLetter.php"><input type="hidden" name="type" value="$type">
273 <input type="hidden" name="recordid" value="$record">
274 <td bgcolor="$color"><input type="submit" value="Contract"></td>
275 </FORM>
276 END_OF_THIRD
277 }
278 else
279 {
280 print "<td bgcolor=$color></td>";
281 }
282
283
284 return 1;
285 }


Here is the code for the sub routine in the library.
Any help would be greatly apreciated.

1212:sub PrintRow
1213-{
1214-
1215-#Try not to use this thing any more -- I want to wax it but we use it in too many places to do that!
1216-($type,$record,@stuff) = @_;
1217-$counter = 0;
1218:print "<!--html-lib.pl::PrintRow-->\n<tr>\n";
1219-foreach $cell (@stuff)
1220- {
1221- if($cell eq "") { $cell = "&nbsp\;"; }
1222- print "<td bgcolor=\"$color\"><font face=\"Arial, Helvetica, sans-serif\" size=\"-1\">$cell</td>\n";
1223-# print "<td bgcolor=\"$color\"><font face=\"Arial, Helvetica, sans-serif\" size=\"-1\">$stuff[$counter]</td>\n";
1224-# $counter++;
1225- }
1226-print<<END_OF_CRAP;
1227-<form method="post" action="/cgi-bin/mbm/ModifySetup.cgi"><input type="hidden" name="type" value="$type">
1228-<input type="hidden" name="recordid" value="$record">
1229-<td bgcolor="$color"><input type="submit" value="Modify"></td>
1230-</FORM>
1231-<form method="post" action="/cgi-bin/mbm/DeleteConfirm.cgi"><input type="hidden" name="type" value="$type">
1232-<input type="hidden" name="recordid" value="$record">
1233-<td bgcolor="$color"><input type="submit" value="Delete"></td>
1234-</FORM>
1235-</tr>
1236-END_OF_CRAP
1237-return 1;
1238-}
1239-


Here is the code for the call to the sub routine;

202 &PrintSal("contractrpt",$rowno,$salyrs,$contr_type,$salary,$base,$va,$incentive_pay,$est_fpc_pay,$est_fpc_from_date,$est_fp c_to_date,$guar_fpcpay,$guar_datefrom,$guar_dateto);
203 $prtupd=0;
204 $prntcontracts = "NO";
205 &PrintSal("salrptsuppl",$row_no,$psuppltype,$psupplamt,$psuppldesc);
206 print "</tr>";
207 $prtupd=1;
208 $ticker++;

[KevinADC]

this is wrong and is probably the reason the button is printed twice:

if (($prntcontracts == "YES") && ($prtupd == 1))

"==" is a numeric operator, but you are checking a string value "YES" so you want to use "eq" instead of "==":

if (($prntcontracts eq "YES") && ($prtupd == 1))

change that and see if it resolves the two button problem.

[KevinADC]

this is just an html comment:

print "<!--html-lib.pl::PrintRow-->\n";

it does nothing as far as the program is concerned. Not sure why it is there in the html code output. The code you posted is calling the PrintSal subroutine, not the PrintRow subroutine. Which is what the html comment above is indicating for some unknown reason.

Judging by thte code you posted, the author was not a good perl programmer. The code indicates a fundamental lack of writing good perl programs. It might work but is a potential can of worms.

[oesxyl]

this is just an html comment:

print "<!--html-lib.pl::PrintRow-->\n";

it does nothing as far as the program is concerned. Not sure why it is there in the html code output.
I guess it use SSI, :)

best regards

[KevinADC]

I guess it use SSI, :)

best regards

that is not a valid SSI tag and while it is possible to use SSI and script generated output its something you rarely see. Maybe there is something else going on that we are unaware of.

[bazz]

Can I chip in first (FishMonger). lol

You really need to make sure that your script starts with the strict pragma.


use strict;

Your sub, in the second code section doesn;t declare the vars as it should.

also, make sure you have warnings outputted to the browser or your server error logs.


use warnings; # at top of script


But, also, you might want to read up on the CGI module since it could make your script shorter and easier to use.

hth

bazz

[FishMonger]

make sure you have warnings outputted to the browser or your server error logs.


use warnings; # at top of script



You forgot something.
use warnings;
use CGI::Carp qw<fatalsToBrowser warningsToBrowser>; # this outputs the errors to the browser, but the warnings needs 1 more step.
After outputing the html header, you add this line which will output the warnings as html comments.
warningsToBrowser(1);

The script is using depreciated html code which would be better handled with the use of css.

Rather than outputting raw html, I'd use HTML::Template. http://search.cpan.org/~samtregar/HTML-Template-2.9/Template.pm