Script help

Deleuze has asked for the wisdom of the Perl Monks concerning the following question:

Hi. This is my first real perl script, and I am somewhat clueless. I intended it to monitor a number of given AIM screennames for activity (logging in, out, etc) and logging this activity to a file. Needless to say, it doesn't run. Correctly, at least. Can anyone help me out? Thanks...

#!/usr/bin/perl -w

use Net::AIMTOC;
use Term::ReadKey;

$version = 0.01;       #The version number
$screenname = "";      #Contains the screenname for logging in
$password = "";        #Password for logging in
$out = "STDOUT";       #Set default output to the screen
@targets;              #Array containing screennames to watch


# process($)
# Takes next argument as parameter.
# Checks to see if it's a flag, otherwise
# considers it a screenname and adds to list.

sub process_args($) {
    my $iterate = 0;     #Local variable for adding to @targets
    if ($_[0] eq "-v" || $_[0] eq "--version") {
        print "Thanks for supporting AimSpy.  You are currently using 
+version $version of AimSpy.\n";
    }
    elsif ($_[0] eq "-h" || $_[0] eq "--help") {
    }               #help info
    elsif ($_[0] eq "-o") {             #check for output file spec.
        $save = shift @ARGV;
        open($out = SAVE, ">>$save") or open($out = SAVE, ">$save") or
                  die "Can't save to file $save: $!\n";
    }
    elsif ($_[0] eq "-i") {             #check for input file spec.
        $read = shift @ARGV;
        open(INPUT, "$read") or die "Could not open file: $!\n";
        $iterate = 0;

        while(<INPUT>) {
            $targets[$iterate] = $_;
            $iterate++;
        }
    }
    else {        #everything else is a screenname
        $targets[$iterate] = $_[0];
        $iterate++;
    }
}


# login()
# Logs in to AIM.  Prompts sn and pw.
# Establishes connection to AOL.
# Doesn't really do too much else....

sub login() {
    if (!@ARGV) {              #We need someone to spy on.
        die "No targets specified!";
    }

    if ($screenname eq "") {
        print "Enter your screenname: ";
        chomp($screenname = <STDIN>);
        print "\nEnter your password: ";
        ReadMode 2;
        chomp($password = <STDIN>);
        ReadMode 0;
    }

    try {
        $aim = Net::AIMTOC->new;
        $aim->connect;
        $aim->sign_on($screenname, $password);
    }
    catch Net::AIMTOC::Error with {
        my $err = shift;
        print $err->stringify, "\n";
    };
    foreach my $iterate (@targets) {
        $aim->add_online_buddies("aimspy", $iterate);
    }
}

sub monitor() {
    while (1) {
        my $msg = $aim->recv_from_aol();

        if ($msg->getType eq 'UPDATE_BUDDY' && $msg->getBuddy eq @targ
+ets) {
            print $out $msg->getBuddy, " turned ", $msg-getOnlineStatu
+s, " at this time.\n";
        }
        elsif ($msg->getType eq 'IM_IN') {
            print $out "Message ", $msg->getMsg, " received from ", $m
+sg->getSender, " at this time.\n";
        }
        elsif ($msg->getType eq 'ERROR') {
            print $out "Error ", $msg->getMsg, " received at this time
+.\n";
        }
    }
}

while (@ARGV) {                 #process arguments
    process_args(shift @ARGV);
}

login;

monitor;
[download]

Comment on Script help Download Code

Replies are listed 'Best First'.

Re: Script help
by demerphq (Chancellor) on Jun 22, 2003 at 17:47 UTC

Im afraid to say that this code is riddled with errors. I dont know how you came to write this, but its very strange indeed.

use Net::AIMTOC;
use Term::ReadKey;

$version = 0.01;       #The version number
$screenname = "";      #Contains the screenname for logging in
$password = "";        #Password for logging in
$out = "STDOUT";       #Set default output to the screen
@targets;              #Array containing screennames to watch
[download]

This is bad. You dont have strictures turned on, and accoridngly are getting warnings for doing dumb things (like put the @targets; all alone on a line like that.) This should read:

use strict;
use warnings;
use diagnostics;
use Net::AIMTOC;
use Term::ReadKey;

my $version = 0.01;       #The version number
my $screenname = "";      #Contains the screenname for logging in
my $password = "";        #Password for logging in
my $out = "STDOUT";       #Set default output to the screen
my @targets;              #Array containing screennames to watch
[download]

But even then $version should change to $VERSION to be compatible with standard convention, and the tools that expect programs to comply with them.

sub process_args($) {
    my $iterate = 0;     #Local variable for adding to @targets
    if ($_[0] eq "-v" || $_[0] eq "--version") {
        print "Thanks for supporting AimSpy.  You are currently using 
+version $v
+ersion of AimSpy.\n";
    }
[download]

Now you prototype a sub for no reason at all, and then use $_[0] directly. This is bad practice in every way. (There are reasons to do both, but until you know what they are its unlikely you satisfy them.) And to add insult to injury you are reinventing a wheel here. You should be using Getopt::Long to do this.

        open($out = SAVE, ">>$save") or open($out = SAVE, ">$save") or
                  die "Can't save to file $save: $!\n";
[download]

This is all wrong. You probably meant something like

  open my $out,">>$save" or die "Can't append to '$save': $!\n";
[download]

If a file doesnt exist to append to then it is created with >>

        $targets[$iterate] = $_[0];
        $iterate++;
[download]

You need to look up pop/push shift/unshift and splice in perlfunc.

push @targets,$item;
[download]

is how we normally write that.

    try {
        $aim = Net::AIMTOC->new;
        $aim->connect;
        $aim->sign_on($screenname, $password);
    }
    catch Net::AIMTOC::Error with {
        my $err = shift;
        print $err->stringify, "\n";
    };
[download]

try{}catch{} is not part of perl. If you are using this construct then it needs to be defined somewhere. Unless it is defined by Net::AIMTOC then you have a real problem here. Perhaps the Exception module is what you are missing.

I think you need to return to the tutorials for a while my friend. Reread perlsyn and perlopentut and stuff and then retry this. Several of the errors and potential errors in the above code would have prevented the program from compiling under strict. For good reason. In simple and somewhat harsh terms, unless you can explain in detail why you need strictures turned off then there is no excuse at all for not using them.

Good luck.

_{<Elian> And I do take a kind of perverse pleasure in having an OO assembly language...

• Update:

Fixed the bug that Aristotle reported about no mentioning $! in the error message from a failed open. Thanks Aristotle}

[reply]
[d/l]
[select]

Re^2: Script help

by Aristotle (Chancellor) on Jun 22, 2003 at 19:34 UTC

open my $out,">>$save" or die "Can't append to '$save'\n";

$!

:)

Makeshifts last the longest.

[reply]

Re: Re^2: Script help

by demerphq (Chancellor) on Jun 22, 2003 at 19:40 UTC

Indeed. I consider it a bug that I left it out. Sorry about that. And thanks. Good eye.

_{<Elian> And I do take a kind of perverse pleasure in having an OO assembly language...}

[reply]
[d/l]

Re: Script help
by castaway (Parson) on Jun 22, 2003 at 17:14 UTC

[reply]

Re: Script help
by dash2 (Hermit) on Jun 22, 2003 at 17:49 UTC

try {...} catch{ ... }

(Unless I am behind the times and it's one of the very latest features, or exported by default from one of your modules.)

Instead, use eval { #try some code }; if ($@) { # catch the error }. There's no support for finally{}.

Oh, and use strict;.
A massive flamewar beneath your chosen depth has not been shown here

[reply]
[d/l]
[select]

Re: Script help
by antirice (Priest) on Jun 23, 2003 at 00:19 UTC

If you look at the documentation for Net::AIMTOC, the way they use the try construct is by doing use Error qw( :try );

Of course, also use strict; use warnings; use diagnostics; to see what's actually going on when you execute this script.

antirice
The first rule of Perl club is - use Perl
The ith rule of Perl club is - follow rule i - 1 for i > 1

[reply]
[d/l]