in reply to Script Review...
I'll assume that the identing looks goofy because you pasted the code in with a combination of space and tab characters (with indent level set to 4), instead of just spaces -- no big deal here, just make sure the copy you present to the interviewer shows proper indentation.
The nesting of if..else... blocks, and the distance between a short else block from the condition that triggers it, are not severely bad here, but they are unnecessary impediments to readability and maintainability. Your handling of the config file could go like this:
If you really want to report on the status of reading the config file (e.g. report unusable lines), just read the file in a short loop, report the results after closing it, then do a loop over the usable lines -- eg:while (<CONF>) { next unless ( /^(.+)\s(http:\/\/\S+)\s+(.+)$/ ); # just skip unusa +ble lines my ( $NAME, $URL, $ALT ) = ( $1, $2, $3 ); $http->reset; if ( $http->request( $URL ) != 200 ) { warn "cannot read from $URL ($NAME)\n"; next; } if ( http->body() =~ /.../ ... ) { my $IMGURL = $1; ... $http->reset; if ( $http->request( $IMGURL ) != 200 ) { warn "drat! tried to get $IMGURL (for $NAME) but couldn't\ +n"; next; } # OK, got the image... and so on... } }
Note that a "my ... " statement only needs parens when you're doing a list of variables. (update: or when you're declaring a single scalar and assigning a value from something that returns a list, e.g. my ($first) = sort @array;)my %name; my %alt; my %bad; while (<CONF>) { if ( /^(.+)\s(http:\/\/\S+)\s*(.+)$/ ) { $name{$2} = $1; $alt{$2} = $3; } else { $bad{$.} = $_; } } close CONF; if ( keys %bad ) { warn "unusable config data at line $_:\n $bad{$_}" for ( sort { $a <=> $b } keys %bad; } warn "Loading ",scalar( keys %alt )," comic sites\n"; for ( keys %alt ) { # ... and so on }
You weren't doing a consistent amount of error-checking and problem-reporting with your different $http->request calls.
You should add pod that describes the app and gives the necessary detail for creating a config file from scratch (or provides the content of a sample config file).
Okay, this is for comics, so I guess the spelling of "$dimention" is supposed to be a joke, right?
|
|---|
| Replies are listed 'Best First'. | |
|---|---|
|
Re: Re: Script Review...
by allolex (Curate) on Oct 25, 2003 at 23:55 UTC |