Steve Bertrand wrote:
>
Tom Phoenix wrote:
>>
Steve Bertrand wrote:
The current setup feels very 'kludgy' to me. The code snip below comes
out of the module. I hope there is enough context to see why I think it
can be improved.
Actually, the snippet you submitted seems pretty straightforward. It
could be improved (what code couldn't?) but it's not in *need* of
improvement, IMHO: It can't become impressively faster or clearer.
What seems kludgy about it? What improvement are you seeking?
Well, I didn't like the fact I was returning an AoA. I have subsequently
changed the code to only allow a single row return, and assign it
directly to $self via a hashref instead:
-- snip --
while (my $service_info = $query->fetchrow_hashref) {
for my $field (keys %$service_info) {
$self->{$field} = $service_info->{$field};
}
}
-- end snip --
Thanks for your response Tom!
Hi Steve.
This doesn't look right at all. Your original code was
Steve Bertrand wrote:
>
> $query->execute;
>
> my @current_row;
> my @total_rows;
>
> while (@current_row = $query->fetchrow_array()) {
> push (@total_rows, [EMAIL PROTECTED]);
> @current_row = ();
> }
>
> return (@total_rows);
which returns a list of database records. Fine. Nothing wrong with
returning a list or arrays if that's the shape your data is.
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.
Getting DBI to return a hash instead of an array is quite a lot slower,
although that may not matter. 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? If
you are expecting only a single record to be returned then you should
only do a single fetch. But it is hard to see the code you have written
taking the place of what went before, which just collected all the data
records and returned them.
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.
Rob
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
http://learn.perl.org/
