STILL not getting hash building.

[bazz]

Hi,


OK I got the has to build as shown below. Now I am struggling to output 'Rooms', just once for all room types as shown in my second post.

$published_rooms{$rooms} = $Value;



Previously I have been using an array for this but I think a hash will be better.

I am trying to build the hash to output

<ul>
<li>Rooms
<ul>
<li>Double</li>
<li>Twin</li>
<li>Single</li>
</ul>
</li>
</ul>


Unlike currently, I only want the word Rooms to show, if there are 'Values'.

the dump is ok but when I process it, the value is a mess. :( error 'HASH(0x9e02654)'


my $dat_file = "$cmsFilesLocation/$business_id/Rooms/roomList.dat";

local *DAT;

my $rooms = 'Rooms';
my %published_rooms;

if ( open ( DAT, "< $dat_file" ) )
{
while ( my $Line = <DAT> )
{
last if ( index($Line, ':') == 1 ); #splits the line at the first : ignoring any others.
chomp $Line;
my ( $Keyword, $Value ) = split /:/, $Line, 2;

$published_rooms{$rooms}{$Value}++;
}

} else { print qq( cant); }

print Dumper \%published_rooms;


foreach my $room (sort keys %published_rooms) {
print "$room: $published_rooms{$room}<br />";
}



bazz

[bazz]

Here's the full sub followed by an attempt, which works but likely is not the most efficient way.


This sub outputs Rooms even if there is none.


my $business_id = shift;
my $dat_file = "$cmsFilesLocation/$business_id/Rooms/roomList.dat";

local *DAT;
#open DAT, "< my $dat_file" || die "couldn't open file\n";
my $rooms = 'Rooms';
my %published_rooms;

if ( open ( DAT, "< $dat_file" ) )
{
print qq(
<li>Rooms
<ul>
);

while ( my $Line = <DAT> )
{
last if ( index($Line, ':') == 1 ); #splits the line at the first : ignoring any others.
chomp $Line;
my ( $Keyword, $Value ) = split /:/, $Line, 2;

if ($Value)
{

print qq(
<li> <a href="/cgi-bin/rooms/Rooms/$Value/$business_id">$dishName</a></li>
);
}
}
print qq(
</ul>
</li>
);

}


This sub snippet shows how I am showing Rooms only if there are some. ( :cool: ) but is it the most efficient way?



if ( open ( DAT, "< $dat_file" ) )
{
my $count=0;
while ( my $Line = <DAT> )
{
if ($count == 0 )
{
print qq(
<li>Rooms
<ul>
);
}
$count++;
last if ( index .....etc


The good news is that, either way, I have already reduce the sub size by 50 lines because I haven't used an array.

bazz

[KevinADC]

Is the file only one line?

last if ( index($Line, ':') == 1 )

that exits the file as soon as it finds a ":" in a line.

[bazz]

Thanks Kevin. It seems to be ok at reading each line individually and splitting on the first :

The hash now builds as shown in the last [ code ] section above. BUt is it necessary to use $count to be sure that Rooms outputs only once? Rooms is the key btw.

bazz

[FishMonger]

Why are you using:
local *DAT;

Instead, you should be using the 3 arg form of open and a lexical var for the filehandle.
if ( open my $DAT, '<', $dat_file )

last if ( index($Line, ':') == 1 );That will come into play if a colon is the second character in the line (the index function is 0 indexed just like arrays). Is that what you intended, or do you expect it to be the first character?

There's no need to create/use $count. Instead, you could/should use the built-in $. var which holds the current line number.

[KevinADC]

Do something like this:

my $business_id = shift;
my $dat_file = "$cmsFilesLocation/$business_id/Rooms/roomList.dat";

local *DAT;
#open DAT, "< my $dat_file" || die "couldn't open file\n";
my $rooms = 'Rooms';
my %published_rooms;
my $output = 0;
if ( open ( DAT, "< $dat_file" ) ) {
while ( my $Line = <DAT> ) {
last if ( index($Line, ':') == 1 ); #splits the line at the first : ignoring any others.
chomp $Line;
my ( $Keyword, $Value ) = split /:/, $Line, 2;
if ($Value){
$output .= qq{<li> <a href="/cgi-bin/rooms/Rooms/$Value/$business_id">$dishName</a></li>};
}
}
}
if ($output) {
print qq{
<li>Rooms
<ul>
$output
</ul>
</li>
};
}

[bazz]

@Kevin, thank you. I'll check that out in more detail after this reply.

Why are you using:
local *DAT;

erm, that was a hangover from an earlier code that I had paid to be done (before finding CF), and assumed was correct. Now no longer used.


Instead, you should be using the 3 arg form of open and a lexical var for the filehandle.
if ( open my $DAT, '<', $dat_file )


OK, I have reverted to the straighforward

open FILE, "$dat_file" or die $!;

since I need only read access.


last if ( index($Line, ':') == 1 );That will come into play if a colon is the second character in the line (the index function is 0 indexed just like arrays). Is that what you intended, or do you expect it to be the first character?


Again a hangover but I have never had a colon as the second character. It is supposed to split the line on the first colon, wherever it is, and to make whatever is after it, the value.


There's no need to create/use $count. Instead, you could/should use the built-in $. var which holds the current line number.

I'll check that out and see where it takes me.

Thank you Fish.

bazz

[bazz]

OK, here it is now and it still works. :)

I don't recall now what the 'last if' line was supposed to do but it is now irrelevant and has been removed.

So is it as efficient as possible?


my $business_id = shift;
my $subject = 'Rooms';
my $dat_file = "$cmsFilesLocation/$business_id/$subject/roomList.dat";

open FILE, "$dat_file" or die $!;

while ( my $Line = <FILE> )
{

if ($. == 1 )
{
print qq(
<li>$subject
<ul>
);
}

chomp $Line;
my ( $Keyword, $Value ) = split /:/, $Line, 2;

if ($Value)
{
my $itemName = $Value;
$itemName =~ s/es_/e's /g;
$itemName =~ s/A_la_C/À la C/g;
$itemName =~ s/e_D_H/e D' H/g;
$itemName =~ s/_/ /g;
print qq(
<li> <a href="/cgi-bin/rooms/$subject/$Value/$business_id">$itemName</a></li>
);
}

}

print $. ;
if ( $. > 0)
{
print qq(
</ul>
</li>
);
}
close FILE;

bazz

[KevinADC]

I think my way might be more efficient but I'm not sure. Your way has to test all lines of the file to see what the line number is even though you only want to find line number one to make sure there is something in the file. That is never efficient but if its a small file there is probably not much difference if any.

[FishMonger]

If something needs to be printed prior to the first line and only then, then move that print statement outside of and prior to the while loop and forget about testing the line number in the while loop.

I also don't see the need to build up the $output var, unless $Value could be undef for each and every line in the dat file i.e., the dat file doesn't hold the data that you expect. Instead of building the $output var, just output the desired link. If you do need to build that var, then the output of it should be put inside the if ( open ( DAT, "< $dat_file" ) ) { block, instead of after it.

I know I've pointed this out a number of times, but you should use the 3 arg form of open and a lexical var for the filehandle and you should read:
perldoc -q quoting

my ( $Keyword, $Value ) = split /:/, $Line, 2;Since $keyword doesn't appear to be used/needed, why create the var?
Do this, instead.
my $Value = (split /:/, $Line, 2)[1];

open FILE, "$dat_file" or die $!;Having $! in the die statement is good in development, but may or may not be good in a production script. Also, the error message is incomplete and where is that error going?
Are you using CGI::Carp qw/fatalsToBrowser/;to redirect that to the user, or do you have a custom die handler?

[bazz]

If something needs to be printed prior to the first line and only then, then move that print statement outside of and prior to the while loop and forget about testing the line number in the while loop.


Not that simple :)
This is for a list containing another list - a flyout menu.
The first list item is to be Rooms and it wil contain a list of actual rooms available. So if there are rooms, I want the word Rooms to output once and if there are no rooms, I do not want that section of the nav menu to show.


I know I've pointed this out a number of times, but you should use the 3 arg form of open and a lexical var for the filehandle and you should read:
perldoc -q quoting


OK, I'll try to get it wokring in the morning with the lexical var as filehandle.


my ( $Keyword, $Value ) = split /:/, $Line, 2;Since $keyword doesn't appear to be used/needed, why create the var?
Do this, instead.
my $Value = (split /:/, $Line, 2)[1];


good point :thumbsup:


open FILE, "$dat_file" or die $!;Having $! in the die statement is good in development, but may or may not be good in a production script. Also, the error message is incomplete and where is that error going?
Are you using CGI::Carp qw/fatalsToBrowser/;to redirect that to the user, or do you have a custom die handler?

I am using it only for testing because later, I will have the errors showing up in the server logs. I don't have a custom die handler because I never even thought about writing one and now, am not sure how to grab it and direct it.

bazz

[FishMonger]

You should take a serious look at:

Log::Log4perl
http://search.cpan.org/~mschilli/Log-Log4perl-1.18/lib/Log/Log4perl.pm

DBIx::Log4perl
http://search.cpan.org/~mjevans/DBIx-Log4perl-0.12/lib/DBIx/Log4perl.pm

Log4perl general search
http://search.cpan.org/search?query=log4perl&mode=all

[bazz]

just a slight amount of reading there Fish. :eek:

Thanks for the advice.

bazz

[FishMonger]

I thought you liked reading. :D

[bazz]

Ha. well if I didn't, I'd better begin to like it. :D