> In your new code you retrieve each record as a hash, keyed by field
> name. Then, instead of your subroutine returning the result as it did
> before, you copy the hash elements directly into your object.

Correct. Essentially what I want is to absolutely minimize any work I
have to do outside of the module. This particular module subroutine will
need to be called from several different applications, so instead of
returning an AoA, then having to process through that in each
application, the more data directly assigned via the subroutine, the
less work I have to do later.

> Getting DBI to return a hash instead of an array is quite a lot slower,
> although that may not matter. 

In this case, it does not.

> But more worryingly you are overwriting
> the object values each time you fetch a record, so that when the loop
> exits only the last set of data is retained. Is that what you want? 

Well, not exactly. I will see either 0, one or two records per call.
It's looking for a user's RADIUS month history, and I will have either
no records, a record for DSL usage, dialup usage or both. See below what
I have done to see if it makes better sense now, or if it can be made
better.

> I would always be hesitant to alter existing code if it works and is
> readable. Only ever change it if it is too slow or incomprehensible.

I agree, however, this is all in the name of minimizing code writing
later (if this makes sense). Here is the entire sub. Hopefully It won't
wrap uncontrollably:

--- snip ---

my $self     = shift();
my $dbh      = shift();
my $username = shift();
my $date     = shift();
my $hs_record = 0;              # HighSpeed row
my $du_record = 0;              # DialUp row

my $query = $dbh->prepare("

    SELECT * FROM mtotacct
    WHERE UserName = '$username'
    AND AcctDate LIKE '$date%';");

$query->execute() or die $dbh->errstr;

# Extract the info from the DB, and assign it to $self

my $hs = 'highspeed';
my $du = 'dialup';

while (my $service_info = $query->fetchrow_hashref) {

    for my $field (keys %$service_info) {

        if ($service_info->{'NASIPAddress'} =~ /208.70/) {
            $self->{$du}->{$field} = $service_info->{$field};
            $du_record = 1;

        } elsif ($service_info->{'NASIPAddress'} =~ /199.166/) {
            $self->{$hs}->{$field} = $service_info->{$field};
            $hs_record = 1;
        }
    }
}

# Return back either a true or false for dialup and highspeed
# I'm wondering at this point if I should just go back to the
# returning of the AoA, as I'm in a situation where I now am
# processing an array again anyhow.

my @records = ($du_record, $hs_record);
return (@records);


--- end snip ---

Thanks,

Steve

-- 
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
http://learn.perl.org/


Reply via email to