[Pdx-pm] Different data structures for Hash of array, same code (mostly)
Shlomi Fish
shlomif at iglu.org.il
Wed Nov 18 08:55:59 PST 2009
Hi Daniel!
On Wednesday 18 Nov 2009 17:12:07 Daniel Herrington wrote:
> All,
>
> Sorry if this is a double post, but I didn't get a confirmaiton that my
> last email made it.
It did.
>
> I have two code snippets that should do the same thing, but do not
> produce the same data structure. I can't seem to figure out why.
>
It seems your code snippets contain many anti-patterns. Let me say what I find
wrong with them. If you wonder why a certain portion of code does not work as
it should, you can always use the perl debugger interactively:
http://perl-begin.org/topics/debugging/
> Code A:
> ...
> if ($boxNameH{$boxName}) {
First I should note that I find these code snippets hard to read. They do not
contain any empty lines, separating code paragraphs, and the variable names
are given in camelCase or CamelCase with weird System hungarian notation (not
even Apps Hungarian - see:
http://en.wikipedia.org/wiki/Hungarian_notation#Systems_vs._Apps_Hungarian
), with a significant leading intendation which makes it hard to follow inside
an email and "#Comments" instead of "# comments".
Now let me continue.
> #If we already have a list created
> my $refBoxName = $boxNameH{$boxName};
> my @derefArray = @$refBoxName;
> $derefArray[0]++;
> push (@derefArray, $jobName);
> $boxNameH{$boxName} = \@derefArray;
OK, why are you dereferencing an array and just replacing it. You can just do
(untested):
<<<<
my $box = $boxNameH{$boxName};
$box->[0]++;
push @$box, $jobName;
>>>
Much easier, shorter and clearer.
I should note that doing $box->[0]++ seems redundant in this case. Are you
counting the number of jobs there? You can easily determine it based on
scalar(@$box).
> $caJIL{$jobName}{"$jilCommand"} = "$jilValue";
Why are you using "$jilCommand" And "$jilValue" in quotes. Normally the quotes
are not needed (they are when you are trying to stringify objects. What is the
meaning of the jil prefix?
> } else {
> #We're the first one
> my @derefArray;
> $derefArray[0] = 1;
> push (@derefArray, $jobName);
> $boxNameH{$boxName} = \@derefArray;
> $caJIL{$jobName}{"$jilCommand"} = "$jilValue";
> }
Again, you don't need to count the number of jobs explicitly.
> ...
> Using Dumper, the data in %boxNameH is:
> 3108$VAR1 = {
> 'CPO_TNC_WCONV0' => [
> 2,
> 'CPO_TNC_WCONVERSION_CHMOD',
> 'CPO_TNC_WCONVERSION_SETUP'
> ],
> 'CURRENTLINK_25' => [
> 1,
> 'CURRENT_LINK_25'
> ],
> ...
>
> Code B:
> ...
> my %newBoxNameH;
> foreach (@arBoxNames) {
> my $boxName = $_;
Why are you doing such a weird loop? Just do:
<<<<
foreach my $boxName (@arBoxNames) {
...
}
>>>>
No need to loop over @arBoxNames with the default variable.
> my $newBoxName;
> my @jobNamesList;
> if ($ref_hlqMap->{$boxName}) {
> $newBoxName = $ref_hlqMap->{$boxName} ; } else {
> $newBoxName = $boxName;
> }
You need to fix your indentation here. Prefarably use:
<<<
$newBoxName = $ref_hlqMap->{$boxName} || $boxName;
>>>
> print __LINE__ . " $newBoxName\n";
Why not use STDERR, or a logging module?
> my $ref_arJobNamesB = $ref_boxNameH->{$boxName};
Implement my commentaries to your code below too.
Both of these code snippets seem entirely different to me at first glance.
Your first one has no loops.
Regards,
Shlomi Fish
> my @arJobNamesB = $ref_arJobNamesB;
> foreach (@arJobNamesB) {
> my $jobName = $_;
> my $newJobName;
> if ($ref_hlqMap->{$jobName}) {
> $newJobName = $ref_hlqMap->{$jobName};
> } else {
> $newJobName = $jobName;
> }
> if ($newBoxNameH{$newBoxName}) {
> my $ref_arJobNames = $newBoxNameH{$newBoxName};
> my @arJobNames = @$ref_arJobNames;
> push (@arJobNames,$newJobName);
> $newBoxNameH{$newBoxName} = \@arJobNames;
> } else {
> my @arJobNames;
> push (@arJobNames,$newJobName);
> $newBoxNameH{$newBoxName} = \@arJobNames;
> }
> }
> ...
> 289$VAR1 = {
> 'CURRENTLINK_25' => [
> [
> 1,
> 'CURRENT_LINK_25'
> ]
> ],
> 'CPO_TNC_WCONV0' => [
> [
> 2,
> 'CPO_TNC_WCONVERSION_CHMOD',
> 'CPO_TNC_WCONVERSION_SETUP'
> ]
> ]
> };
> ...
>
> You can see in Code B's dump that the list is actually embedded another
> layer down as if Code B produces a Hash of Array of Array. I just can't
> seem to figure out why this is happening?
>
> thanks,
>
--
-----------------------------------------------------------------
Shlomi Fish http://www.shlomifish.org/
Optimising Code for Speed - http://shlom.in/optimise
Chuck Norris read the entire English Wikipedia in 24 hours. Twice.
More information about the Pdx-pm-list
mailing list