Hi. I have some code here that I am taking to an interview as an example piece of code, so I thought I would post it on here and let the monks give it a look over first.
If you want to actually run it I recommend getting it from here because it comes with example configs and things.
The function of this thing is to get comics off the internet (using http::lite) and display them in a Tk window.
Basically. What could I have done better? How would you critisise my code? What do you think?
#!/usr/bin/perl -w # getcomic - gets my comics # Joe Higton <joe@draxil.uklinux.net> # (Release) V0.4.1 ######################################################### # Config file -- (UNIX)alikes ~/.getcomic # (WIN) whatever $ENV{APPDATA} is on your machine (shou +ld be under you documents and settings dir under appdata as long as +your on NT # Config file takes format # comicname comicpageurl somethingtomatch # # somethingtomatch is something identifiable in your comics IMG tag. N +ormally the alt will do but if there is no ALT then something else, l +ike a non-changing dir name or the image name it's self if this doen' +t change.. # An example config should come with this file, if not goto: # http://www.draxil.uklinux.net/hip/index.pl?page=get_comics and get a + proper one. # if your using crazy/old screen resolutions you may wish to alter %de +faultsize # remember, if this script kills your system it's not my fault. After +all you can read the code: use Tk; use Tk::Image; use Tk::Photo; use Tk::Pane; use Tk::Wm; use MIME::Base64; use strict; use HTTP::Lite; use English; #global mys my($http) = new HTTP::Lite; my($window) = MainWindow->new(); my($comicpane) = $window->Scrolled('Pane',-scrollbars=>"oe"); my(%agoodsize)=( '-width'=>0, '-height'=>0 ); #what we use to decide h +ow big our inital pane will be my(%defaultsize)=( '-width'=>800, '-height'=>600 ); #don't let users resize horizontally, we set the width on the basis of + the comic. $window->resizable(0,1); ### # open our config file based on OS.. # NOTE: Currently we assume (windows || unix). # If I can test this on other platforms I will add other exceptions. if ($OSNAME =~/win/i){ # Windows and therefore, no home dir open(CONF,"$ENV{APPDATA}\\getcomic") or die "can't open config fil +e$ENV{APPDATA}\\getcomic" ; $RS="\n"; # I am told this is needed for activestate } else{ # Yay! Tastes like UNIX, so try using a unix dotfile for the confi +g. open(CONF,"$ENV{HOME}/.getcomic") or die "can't open config $ENV{H +OME}/.getcomic"; } # for each entry in the config file.. while(<CONF>){ chomp; if(/^(.+)\s(http:\/\/\S+)\s*(.+)$/){ my($NAME)=$1; # The name of the comic. my($URL)=$2; # The URL with the page that the comic image is in my($ALT)=$3; # The ALT tag text OR image fragment ## Retrieve the document. $http->reset; my($req)=$http->request($URL); if ($req==200){ ## Check for match text within an img tag. # This is probably in the alt text or a fragment of the SRC ur +l.. if( $http->body()=~/<\s*img[^>]*src\s*=\s*[\'\"]([^\'\"]+)[\'\" +][^>]*$ALT/i || $http->body()=~/<\s*img[^>]*src\s*=\s*[\'\"]([^\'\"]*$ALT[^ +\'\"]*)[\'\"]/i || $http->body()=~/<\s*img[^>]*$ALT[^>]*src\s*=\s*[\'\"]([^\'\ +"]+)[\'\"]/i ) { my($IMGURL)=$1; if ($IMGURL!~/^http:\/\//){ #if we got a relative address try and attach the domain we + got from the page URL $URL=~/(http:\/\/\w+\.\w+\.\w+)/; $IMGURL=$1.$IMGURL; } ##retreive image $http->reset; if(my($req)=$http->request($IMGURL)){ my($raw_im, $lb_im); ##Ok.. We have got an image so add everything to the GUI. $comicpane->Label( -text=>$NAME, -font=>'*-24')->pack(-anc +hor=>'w'); $raw_im=encode_base64($http->body()); $comicpane->Photo($NAME,-data=>$raw_im,-format=>'gif'); $lb_im=$comicpane->Label(-image=>$NAME); $lb_im->pack(-anchor=>'w'); #keep track of our biggest comic $agoodsize{'-width'}=$lb_im->width if($lb_im->width > $ago +odsize{'-width'}); #... and height of our comic stack $agoodsize{'-height'}=$lb_im->height+$lb_im; } } } else{ print STDERR "Could not retrieve $2 ($1)\n"; } } else{ print STDERR "We seem to have an invalid config entry:\n $_" if $_ +; } } exit(0) if $agoodsize{'-height'}==0; # we got nothing, we go no furthe +r.. $comicpane->pack(-fill=>'both',-anchor=>'w',-expand=>1); ##figure out what size our comic pane should be. my ($dimention); for $dimention ('-width','-height'){ if($agoodsize{$dimention}>$defaultsize{$dimention}) { $comicpane->configure($dimention=>$defaultsize{$dimention}); } else { $comicpane->configure($dimention=>$agoodsize{$dimention}); } } ##leave the user in Tk's capable hands! $window->MainLoop;
update (broquaint): added <readmore> tag
(update) Clarification! By the way, I am not just getting you guys to perfect my work, which would obviously be quite dishonest, but I am instead trying to prepare myself for the kind of questions and critisisms I may recieve..
In reply to Script Review... by draxil42
| For: | Use: | ||
| & | & | ||
| < | < | ||
| > | > | ||
| [ | [ | ||
| ] | ] |