> please have a look at the code below and give comments
Here are some quick comments.
#1. Always "use strict"
#2. See #1.
When you "use strict" it foeces you to do things the "right way" and will
help catch errors because of the extra checks it makes.
So something like this:
> @dataFile=<>; # read in file from command line
Needs to be changed to this by explicitly declaring that variable:
my @dataFile=<>; # read in file from command line
> @standardRules=`cat standard.for.arf.txt` ;
This isn't portable (if you care for it to be), and does not check for
errors. This might be better:
open IN, 'standard.for.arf.txt' or die $!;
my @standardRules = <IN>;
close IN;
> (@sitePVCs)=split(/;/,$siteAllPVCs,$siteNoOfPVCs);
The "(" and ")" force list context. The array @sitePVCs will already force
list context without the parens. This can be rewriten like this, which may
or may not be more readable to you:
my @sitePVCs = split(/;/,$siteAllPVCs,$siteNoOfPVCs);
> open(ARFfile, ">$siteARFfile") or die("can not open
Typically filehandles are in all caps. They don't need to be, but it is the
usual way of doing things because it makes them easier to spot (especially
to people other than the author). Also the parens are not needed because
"or" has very low precedence. I also tend to put my error condition on the
next line, but that is just my preference.
open ARFFILE, ">$siteARFfile"
or die "can not open '$siteARFfile': $!";
Again, parens not needed here, but they don't hurt either:
> print ARFfile ("@standardRules\n"); #print standard bits
print ARFFILE "@standardRules\n"; #print standard bits
This is pretty icky:
> print ARFfile ("name matches \".*RH-Serial.*\":
> {\n \tsetName(\"$SiteName-RH-WAN\$2\") ;\n \tsetGroup
> (\"$siteGroup\") ..<snip>.."); # print RH-Serial rule
Try a here-document instead:
# print RH-Serial rule
print ARFFILE <<EOF;
name matches ".*RH-Serial.*": {
setName("$SiteName-RH-WAN\$2");
setGroup("$siteGroup");
setAlias("$siteCCTReff");
setSpeedIn("$siteACRate");
setSpeedOut("$siteACRate");
setDeviceSpeedIn("$siteACRate");
setDeviceSpeedOut("$siteACRate");
}
EOF
It makes it a lot easier to read, not to mention I could remove the \n and
the \" escapes. BTW - If you have quotes in your string you can do this
qq[a "blah" b] instead of "a \"blah\" b". The char following the "qq" can
be any char, so you could use qq{}, qq||, qq**, etc.
In general there isn't anything *wrong* with the script... but "use strict"
is STRONGLY encouraged. The rest are just suggestions for readability.
Rob
-----Original Message-----
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]
Sent: Thursday, November 06, 2003 11:34 AM
To: [EMAIL PROTECTED]
Subject: RFC on first perl script
Hi all
well im trying at lerning this perl stuff.. reading from the "learning
perl" oreilly book and a few other places,
but also using perl a long time before i should ie making the below script,
so that i dont get in to any very bad habbits at such an early stage.
could some one please have a look at the code below and give comments, the
code does do what i want it to ( well at leest from this stage).
#!/usr/bin/perl -w
############################################################
# this function creates a arf rule file from an input file
# Version 0.1 6/11/03
############################################################
@dataFile=<>; # read in file from command line
@standardRules=`cat standard.for.arf.txt` ;
foreach $site (@dataFile) { # loop for each line/site in dataFile
chomp $site;
($siteLink,$siteNoOfPVCs,$siteAllPVCs)=split(/:/,$site,3); #split
up main / pvc info
($siteIP,$siteString,$SiteName,$siteGroup,$siteCCTReff,$siteACRate)=split(/,
/,$siteLink,6);
#split up main info
(@sitePVCs)=split(/;/,$siteAllPVCs,$siteNoOfPVCs);
my $siteARFfile = "$siteIP.arf";
open(ARFfile, ">$siteARFfile") or die("can not open
'$siteARFfile': $!");
print ARFfile
("######################################################################
\n# \n# Discover Rule for:
$siteIP \n#
\n######################################################################
\n\n"); # print header
print ARFfile ("@standardRules\n"); #print standard bits
print ARFfile ("name matches \".*-Cpu-.*\": {\n \tsetName
(\"$SiteName-RH-Cpu\") ;\n \tsetGroup (\"$siteGroup\")
;\n \tsetAlias (\"RH-Cpu\") ;\n} \n\n" ); # print -Cpu- rule
print ARFfile ("name matches \".*-RH\": { \n \tsetName
(\"$SiteName-RH\") ;\n \tsetGroup (\"$siteGroup\") ; \n \t
setAlias (\"RH\") ;\n} \n\n" ); # print -RH rule
print ARFfile ("name matches \".*RH-Serial.*\": {\n \tsetName
(\"$SiteName-RH-WAN\$2\") ;\n \tsetGroup (\"$siteGr
oup\") ;\n \tsetAlias (\"$siteCCTReff\") ;\n \tsetSpeedIn
(\"$siteACRate\") ;\n \tsetSpeedOut (\"$siteACRate\") ;\n \tsetD
eviceSpeedIn (\"$siteACRate\") ;\n \tsetDeviceSpeedOut (\"$siteACRate\")
;\n} \n\n"); # print RH-Serial rule
print ARFfile ("name matches \".*-Serial.*\": {\n \tsetName
(\"$SiteName-WAN\$2\") ;\n \tsetGroup (\"$siteGroup\"
) ;\n \tsetAlias (\"$siteCCTReff\") ;\n \tsetSpeedIn (\"$siteACRate\") ;\n
\tsetSpeedOut (\"$siteACRate\") ;\n \tsetDevice
SpeedIn (\"$siteACRate\") ;\n \tsetDeviceSpeedOut (\"$siteACRate\") ;\n}
\n\n"); # print -Serial rule
for ($i = 0; $i < $siteNoOfPVCs ; $i++ ) { # loop for each PVC
($PVCdlci,$PVCname,$PCVreff,$PVCcir)=split(/,/,"$sitePVCs[$i]",4);
# split out pvc info
print ARFfile ("name matches \".*-dlci-$PVCdlci\": {\n
\tsetName (\"$SiteName-$PVCname\") ;\n \tsetGroup
(\"$siteGroup\") ;\n \tsetAlias (\"$PCVreff\") ;\n \tsetSpeedIn
(\"$PVCcir\") ;\n \tsetSpeedOut (\"$PVCcir\") ;\n \tsetDev
iceSpeedIn (\"$PVCcir\") ;\n \tsetDeviceSpeedOut (\"$PVCcir\") ;\n}
\n\n"); # print PVC rules
}
close(ARFfile) or die("can not close '$siteARFfile': $!");
}
---
fnord
yes im a Concord Engineer, no it never flown!
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
