Re: Passing a reference from a subroutine.

[muba]

Wow. Just... wow

My brain exploded several times while reading your code. First of all, there was the suboptimal indentations (just two spaces? Really?), next there was this long, long list of variable assignments some of which seem to have some sort of default fallback value, whereas others don't. But as a whole, it certainly wasn't easy on the eyes. So I copy/pasted your code into my favourite editor to make it more readable.

I stumbled across my $data = shift; — so I made a mental note to remember $data - it bears special meaning, because it will hold the argument given to this subroutine.

Then I saw this immensely long list of variable declarations and my brain just stopped right there. "No," it said. "I ain't gonna do this. I ain't even pretending I'm able to keep track of so many variables at once."

"Calm down," I said, "how bad can it be? We've done worse, brainy-brain."

"No," my brain said, sobbing, "look at it. my $$one_num? What the h*** does that even mean? Are we dereferencing a variable that don't even exists yet? And... the naming convention, is there even one? $new_name and $todays_date, but $passdate and $thetotal? It's a mess, dude. A mess. Seriously, I ain't not doing it." Then it made some sort of defensive, hands-off kind of gesture and turned its back at me. I think it was serious about not wanting to do it.

But I had to say my brain was having a point there. Anyway, I read a little more of your code, and, curious as it is, my brain peeked along, and I swear I could feel it shed a tear when we noticed each and every one of those assignment statements (except for the last one) ended in a comma. Why? What does it signify? How does it even work? What would've been wrong with semicolons in those places? Why is there this long list of my (...) near the top of this subroutine that doesn't even word-wrap prettily while you could've just gone my $one_num = $infs->{ $one_num }; ... Wait ... what?

My brain was loud when it vocalized its next protest. "Where the h*** does that last $one_num even come from? The one inside the curly braces? The only thing we've assigned so far is $data! There ain't no $one_num! And look! Just look, dude! He does it again in the next line, with $infs->{$account_num}. There ain't no $account_num! It holds no value that I know of!" It looked at me, desperately, and stated ever so clearly, "Ain't. Gonna. Do it."

It added a dramatic silence, before it elaborated, "$one_num and $account_num? They're like the magician's rabbits, coming out of this hat and everything. Look, a rabbit is there, or it ain't there. There ain't no such thing as a rabbit not being in a hat, yet still coming out of that hat."

Still, I continued, reading, and basically what I extracted was that you're trying to build an array out of certain elements that you took from the hash ref $infs. You do this by pushing onto @AoA... What?

Where does @AoA even come from? Another rabbit? Oh, wait, no. It's hidden near the top, right there on the line my $c = -1; my @AoA;. I see what you did there — sleigh of hand. While you're amazing your audience with pulling rabbits with one hand, your other hand sneakily tries to hide a variable behind another one. Really clever. Good move, too, to throw us of our tracks, but I caught you, haha!

Still, I had my hopes up that I would be able to make it more readable, even though I wasn't sure what I would do about these $one_num and $account_num rabbits. I decided to leave them for what they were for now, as is, and deal with them later, and this is what I came up with.

# Code readability improvement - phase 1
sub get_data {                  
    my $data = shift;
    
    my $c = -1;
    my @AoA;
    
    foreach my $infs ( @{$data->{ info }} ) {
        $c++;
        my $one_num     = $infs->{ $one_num };
        my $account_num = $infs->{ $account_num };
        my $name        = $infs->{ name };
        my $new_name    = $infs->{ new_name }       || ' ';
        my $plano       = $infs->{ plano }          || ' ';
        my $passdate    = $infs->{ passdate };
        my $todays_date = $infs->{ todays_date };
        my $descont     = $infs->{ descont };
        my $firm        = $infs->{ firm };
        my $value       = $infs->{ value };
        my $thetotal    = $infs->{ premium };
        my $commited    = $infs->{ commited };

        push @{ $AoA[$c] },$$one_num, $$account_num, $name, $new_name,
+ $plano, $passdate, $todays_date, $descont, $firm, $value, $thetotal,
+ $commited;
   
    }
    
    #print Dumper \@AoA;
    return \@AoA;
 
 }
 
 my $alldata= get_data();
 
 print Dumper $alldata;

"Oh," said my brain, "it's starting to look like something. Still, this seems like a whole lot of lines for something incredibly simple, don'tcha think? Look, there's something there called $value &mdash as opposed to what? The rest of those variables don't represent certain values? And I don't like none of that push ... line none. It's painful to watch, buddy."

"Hm hm," I murmered, as I silently pointed at the my $alldata= get_data() line, especially the part between the parens, where no arguments were given, even though we had taken a mental note to remember the $data variable which would hold the argument given to the subroutine. "I'll show you 'painful'," I thought to my brain, which, being my brain, is able to tell what I'm thinking without me having to tell it what I'm thinking. Which, I think, is quite awesome.

It hurt when I felt how my brain was fighting the urge to just go with its internal Error/Reset/Retry routine. "All this pain to do all that... stuff... that we just had to read through... with NOTHING?" It cried. I tried to comfort it, but it began to tell a little anecdote. "It's like when you were a kid and had to do chores for the whole village for two weeks so you could have five dollars so you could go to the circus, and you had to get there by walking uphill for three hours with the wind blowing in your face. And rain. Oh yeah, lots of cold harsh rain smashing against the soft tender flesh of your face, turning your skin raw and burning. But at last you arrive! And you know what happens then?"

"What happens then?" I asked, dreading I already knew the answer, because wasn't my brain just recalling one of my own memories?

"There ain't no circus. There never ever even been no circus." It sighed. "Seriously. Next time I say I ain't it, you oughta listen."

But we had come so far, I felt we could at least just try to turn this mess into something that at least made a little sense, so I came up with the following:

# Code readability improvement - phase 2
sub get_data {                  
    my $data = shift;
    
    my @AoA;

    
    my @keys_that_I_care_about = qw(
        one_num     account_num name        new_name    plano
        passdate    todays_date descont     firm        value
        thetotal    commited
    );
    
    my @keys_that_need_a_default_value = qw(
        new_name        plano
    );
    
    my $default_value = " ";
    


    
    foreach my $infs    ( @{$data->{ info }} ) {
        my %infs_copy;
        
        $infs_copy{$_} = $infs->{$_}        for @keys_that_I_care_abou
+t;
        $infs_copy{$_} = $default_value     for @keys_that_need_a_defa
+ult_value;

        push @AoA, [ @infs_copy{@keys_that_I_care_about} ];
    }
   
    #print Dumper \@AoA;
    return \@AoA;
 
 }
 
 my $alldata= get_data();
 
 print Dumper $alldata;

"Wow," said my brain, "is this essentially the same code?"

I nodded, "yes. Except I've taken out those $one_num and $account_num rabbits. I hope they were meant as key names anyway instead of variables. But if they weren't, then I'm still gonna pretend they were, for my own sanity."

"Yeah," said my brain. "This is something that looks remotely mentally parse-able."

"Hm hm," I murmured. Not wanting to send my brain to its personal hell again, I resisted the urge to point at the line that called the subroutine with no arguments. It certainly looked better now, but it still wasn't doing a darn thing.