Tom Phoenix wrote:
> On 7/2/07, Mathew <[EMAIL PROTECTED]> wrote:
> 
>> foreach my $date (@searchDate) {
>>         while (my $ticket = $tix->Next) {
> 
> Seeing this worries me. I don't know enough about what's going on to
> tell whether it's wrong or not, but it looks wrong. When the outer
> loop goes on to the second iteration, what does $tix->Next yield? (You
> could use the debugger to find out.)
> 

Yeah, I don't use the most optimal method of iterating through all of
this.  $tix is a reference to a hash of ticket objects.  The ->Next
method simply places each successive object into $ticket for further
processing by other methods.

>> # Format the time spent on each ticket as hh:mm
>> foreach my $user (keys %tickets) {
>>         foreach my $env (keys %{ $tickets{$user} }) {
>>                 foreach my $tikID (keys %{ $tickets{$user}{$env} }) {
>>                         foreach my $subject (keys %{
>> $tickets{$user}{$env}{$tikID} }) {
> 
> You're doing the same dereferencing, just getting deeper each time.
> You'll save some time if you make some of those into temporary
> variables. That fourth nested loop could look more like this, but
> possibly with better variable names:
> 
>        my $subjects_hash = $envs_hash->{$env};
>        foreach my $subject (keys %$subjects_hash) {
> 

I'll see what I can do with this.  It looks simple enough and as I'm now
working my way through Intermediate Perl ( ;) ) I might actually be able
to grasp it :)

>>                                 my @endTime;
>> #                               my $temp          =
>> $tickets{$user}{$env}{$tikID};
>>                                 my $temp          =
>> $tickets{$user}{$env}{$tikID}{$subject};
>>                                 my $temp2         = $temp / 60;
>>                                 my @temp          = split /\./, $temp2;
>>                                 $endTime[0]       = $temp[0];
>>                                 $endTime[1]       = $temp % 60;
>> #                               $tickets{$user}{$env}{$tikID} =
>> sprintf '%d:%02d', @endTime[0,1];
>>                                
>> $tickets{$user}{$env}{$tikID}{$subject} = sprintf '%d:%02d',
>> @endTime[0,1];
> 
> That looks like a difficult way to do it. Maybe something like this?
> 
>          $subjects_hash->{$subject} =
> time_format($subjects_hash->{$subject});
> 
> sub time_format {
>    my $arg = shift;
>    return sprintf '%d:%02d', int($arg / 60), $arg % 60;
> }
> 
> That code assumes, as did the original, that the number to be
> formatted is a non-negative integer of seconds or minutes.
> 

Good assumption :)

> Good luck with it!
> 
> --Tom Phoenix
> Stonehenge Perl Training
> 

Thanks

Mathew
Keep up with my goings on at http://theillien.blogspot.com

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


Reply via email to