Agnello George wrote:
Hi
Hello,
I have written a small script to input customer data in to a flat file, i
would like some guidance on how i could write this script better ,
Define "better".
i would
also like to know if i have optimally used HoH properly .
Can't tell without seeing the actual data you are working with.
-----------------------------------------------------------------------------------------
[r...@localhost scripts]# cat data_insert.pl
#!/usr/bin/perl -w
The -w option is superfluous in this case. Do you know what it is
supposed to do here?
perldoc perllexwarn
# this is a test script trying to play around with perl
# A. ask for name , age, address, emailID phone number and put data in
flat file if email iD exist then exit ;
use warnings;
use strict;
use Data::Dumper ;
my $txt = '/scripts/data.txt';
my %read_data =&get_data;
This would probably be better written as:
my %read_data = get_data( $txt );
You also might want to consider passing around a reference to a hash
instead of the hash itself if the hash contains a lot of data.
print Dumper ([\%read_data]);
# on print of %read_data here is the output
##$VAR1 = [
# {
# '[email protected]' => {
# 'name ' => ' tommy A',
# 'emailid ' => ' [email protected]',
# 'address ' => ' street 123 NY US ',
# 'age ' => ' 33'
You should probably trim leading and trailing whitespace from your data.
# },
# '[email protected]' => {
# 'name ' => ' agnello',
# 'emailid ' => '
[email protected]',
# 'address ' => ' c 123 andheri
mumbai',
# 'age ' => ' 25'
# }
# }
# ];
#
my %database;
$/="\n";
You are setting the Input Record Separator to its default value but you
shouldn't have to do this.
my @allask = qw(name age address emailid );
foreach my $ask (@allask) {
print " please enter $ask:";
while ( chomp($database{$ask} =<STDIN>) ) {
Why do you think you need a hash here? It looks like a simple scalar
would suffice. While that may work correctly, the use of chomp in a
conditional is not usually done. That is usually written as:
while ( $database{ $ask } = <STDIN> ) {
chomp $database{ $ask };
Because the readline like that has an implied defined test.
if ( $read_data{$database{$ask}}{'emailid '} ) { # if emailid exist
Supposedly %read_data only contains 'emailid' keys but here you are
using 'name', 'age' and 'address' as the key which doesn't make sense
and will autovivify those keys into your hash.
print "$database{$ask} already exist \n";
exit;
}
last if $database{$ask};
print "please enter $ask: ";
You could probably rewrite this loop so you don't need to print the same
prompt twice.
}
}
open(OUTFILE,">>","$txt") or die " cannot open file : $!";
(See below.)
foreach (@allask) {
print OUTFILE "$_ : $database{$_}\n";
}
print OUTFILE "\n";
close (OUTFILE);
#--------------------
sub get_data () {
You only really need to use a prototype if you are trying to imitate a
built-in function, they are not really useful for user defined subroutines.
http://perlmonks.org/?node_id=861966
$/="";
If you localize the change to the Input Record Separator here you won't
have to modify it elsewhere.
local $/ = '';
my %hoh = ();
open(IN1,"<","$txt" ) or die " cannot opne file $!" ;
"$txt" makes a copy of $txt but you don't really need a copy here.
perldoc -q quoting
You should probably also use a lexical variable for the filehandle and
you should probably also use the file name in the error message:
open my $IN1, '<', $txt or die " cannot open '$txt' $!";
my $ass;
while ( $ass =<IN1>) {
The contents of $ass do not need to be visible outside of the while loop
so that is usually written as:
while ( my $ass = <$IN1> ) {
chomp ($ass);
my ($email) = reverse split (/:/, "$ass" ) ;
That is usually written as:
my $email = ( split /:/, $ass )[ -1 ];
And again:
perldoc -q quoting
$email =~ s/\s+//g;
foreach (split (/\n/,$ass) ) {
$hoh{$email}{$1} = $2 if /(.*):(.*)$/ ;
You should probably also remove all leading and trailing whitespace here.
}
}
return %hoh;
close(IN1);
}
John
--
Any intelligent fool can make things bigger and
more complex... It takes a touch of genius -
and a lot of courage to move in the opposite
direction. -- Albert Einstein
--
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
http://learn.perl.org/
