Generating a HoH without duplicate code

[lynn]

Hi All,

I am tring to generate a HoH's without using duplicate code. (see script)
Can someone help me do the same thing without the duplicate code

Thanks!!

Lynn


use strict;
use warnings;
use Data:umper;

my %product = ();

while (<DATA>) {
my ( $family, $hour ) = split(/,/);

for ($family) {
if (/NX/) {
$product{'NX'}{'hour'} += $hour;
$product{'NX'}{'count'}++;
} # do something
elsif (/UNIGRAPHICS_NX/) {
$product{'NX'}{'hour'} += $hour;
$product{'NX'}{'count'}++;
}
elsif (/TC_ENGR-IMAN/) {
$product{$family}{'hour'} += $hour;
$product{$family}{'count'}++;
}
elsif (/SOLID_EDGE/) {
$product{$family}{'hour'} += $hour;
$product{$family}{'count'}++;
}
elsif (/WEBTOOLS/) {
$product{$family}{'hour'} += $hour;
$product{$family}{'count'}++;
}
elsif (/TC_COMMUNITY/) {
$product{$family}{'hour'} += $hour;
$product{$family}{'count'}++;
}
else {
$product{'OTHER'}{'hour'} += $hour;
$product{'OTHER'}{'count'}++;
} # default
}
}

print Dumper( \%product );

__DATA__
NX,4.098
UNIGRAPHICS_NX,2.009
WEBTOOLS,.98
TC_ENGR-IMAN,2.09
FEMAP,0.54
SOLID_EDGE,1.03
TC_COMMUNITY,9.02
FEMAP,.81
TC_ENG-META,1.07
NX,1.308
TC_ENGR-IMAN,4.39

 

[Paul Lalli]

I am tring to generate a HoH's without using duplicate code. (see script)
Can someone help me do the same thing without the duplicate code

use strict;
use warnings;

Thank you!!

use Data:umper;

my %product = ();

while (<DATA>) {

Whoa! Thank you even more!!

my ( $family, $hour ) = split(/,/);

for ($family) {
if (/NX/) {
$product{'NX'}{'hour'} += $hour;
$product{'NX'}{'count'}++;
} # do something
elsif (/UNIGRAPHICS_NX/) {
$product{'NX'}{'hour'} += $hour;
$product{'NX'}{'count'}++;
}
elsif (/TC_ENGR-IMAN/) {
$product{$family}{'hour'} += $hour;
$product{$family}{'count'}++;
}
elsif (/SOLID_EDGE/) {
$product{$family}{'hour'} += $hour;
$product{$family}{'count'}++;
}
elsif (/WEBTOOLS/) {
$product{$family}{'hour'} += $hour;
$product{$family}{'count'}++;
}
elsif (/TC_COMMUNITY/) {
$product{$family}{'hour'} += $hour;
$product{$family}{'count'}++;
}
else {
$product{'OTHER'}{'hour'} += $hour;
$product{'OTHER'}{'count'}++;
} # default
}

I would simply group the possible keys into as few if statements as
possible, and set a flag that tells which key should be used:

for ($family) {
my $key;
if (/^(NX|UNIGRAPHICS_NX)$/){
$key = 'NX';
} elsif (/^(TC_ENGR-IMAN|SOLID_EDGE|WEBTOOLS|TC_COMMUNITY)$/){
$key = $family;
} else {
$key = 'OTHER';
}
$product{$key}{'hour'} += $hour;
$product{$key}{'count'}++;
}


(It looks like the NX key could just as easily be grouped in the 2nd if
block too, but I'm just trying to duplicate your logic here)

}

print Dumper( \%product );

Hope this helps,
Paul Lalli

 

[lynn]

Hi Paul,

Paul Lalli wrote:
(snipped)

I would simply group the possible keys into as few if statements as
possible, and set a flag that tells which key should be used:

for ($family) {
my $key;
if (/^(NX|UNIGRAPHICS_NX)$/){
$key = 'NX';
} elsif (/^(TC_ENGR-IMAN|SOLID_EDGE|WEBTOOLS|TC_COMMUNITY)$/){
$key = $family;
} else {
$key = 'OTHER';
}
$product{$key}{'hour'} += $hour;
$product{$key}{'count'}++;
}


(It looks like the NX key could just as easily be grouped in the 2nd
if block too, but I'm just trying to duplicate your logic here)

Wow, That looks good Thanks Paul

Lynn

 

[xhoster]

Hi All,

I am tring to generate a HoH's without using duplicate code. (see script)
Can someone help me do the same thing without the duplicate code

Thanks!!

Lynn

use strict;
use warnings;
use Data:umper;

my %product = ();

while (<DATA>) {
my ( $family, $hour ) = split(/,/);

for ($family) {
if (/NX/) {
$product{'NX'}{'hour'} += $hour;
$product{'NX'}{'count'}++;
} # do something
elsif (/UNIGRAPHICS_NX/) {
$product{'NX'}{'hour'} += $hour;
$product{'NX'}{'count'}++;
}

You can get rid of this elsif altogether. If /NX/ doesn't match, then
/UNIGRAPHICS_NX/ isn't going to, either.

Or maybe you should be testing for equality rather than using regex?:
if ($_ eq 'NX') {

elsif (/TC_ENGR-IMAN/) {
$product{$family}{'hour'} += $hour;
$product{$family}{'count'}++;
}
elsif (/SOLID_EDGE/) {
$product{$family}{'hour'} += $hour;
$product{$family}{'count'}++;
}
elsif (/WEBTOOLS/) {
$product{$family}{'hour'} += $hour;
$product{$family}{'count'}++;
}
elsif (/TC_COMMUNITY/) {
$product{$family}{'hour'} += $hour;
$product{$family}{'count'}++;
}

Since these all do the same thing, just combine the conditions with OR into
one if condition.

elsif (/TC_ENGR-IMAN/ or /SOLID_Edge/ or /WEBTOOLS/ or /TC_COMM/) {
$product{$family}{'hour'} += $hour;
$product{$family}{'count'}++;
}

else {
$product{'OTHER'}{'hour'} += $hour;
$product{'OTHER'}{'count'}++;
} # default
}
}

Xho

 

[Anno Siegel]

Hi Paul,

Paul Lalli wrote:
(snipped)

Wow, That looks good Thanks Paul

One nit to pick: Like your original code, it refers to the variable
$family where the loop variable $_ should be used. The pattern matching
refers implicitly and correctly to $_. These happen to have the same
value through the one and only run of the loop, but it makes the code
more fragile than it has to be.

Anno

 

[John W. Krahn]

I am tring to generate a HoH's without using duplicate code. (see script)
Can someone help me do the same thing without the duplicate code


use strict;
use warnings;
use Data:umper;

my %product = ();

while (<DATA>) {
my ( $family, $hour ) = split(/,/);

for ($family) {
if (/NX/) {
$product{'NX'}{'hour'} += $hour;
$product{'NX'}{'count'}++;
} # do something
elsif (/UNIGRAPHICS_NX/) {
$product{'NX'}{'hour'} += $hour;
$product{'NX'}{'count'}++;
}
elsif (/TC_ENGR-IMAN/) {
$product{$family}{'hour'} += $hour;
$product{$family}{'count'}++;
}
elsif (/SOLID_EDGE/) {
$product{$family}{'hour'} += $hour;
$product{$family}{'count'}++;
}
elsif (/WEBTOOLS/) {
$product{$family}{'hour'} += $hour;
$product{$family}{'count'}++;
}
elsif (/TC_COMMUNITY/) {
$product{$family}{'hour'} += $hour;
$product{$family}{'count'}++;
}
else {
$product{'OTHER'}{'hour'} += $hour;
$product{'OTHER'}{'count'}++;
} # default
}
}

print Dumper( \%product );

__DATA__
NX,4.098
UNIGRAPHICS_NX,2.009
WEBTOOLS,.98
TC_ENGR-IMAN,2.09
FEMAP,0.54
SOLID_EDGE,1.03
TC_COMMUNITY,9.02
FEMAP,.81
TC_ENG-META,1.07
NX,1.308
TC_ENGR-IMAN,4.39

You could do it with a regular expression:

my %product;
while ( <DATA> ) {
if (
/^(?:UNIGRAPHICS_)?(TC_COMMUNITY|WEBTOOLS|SOLID_EDGE|TC_ENGR-IMAN|NX)?(?:.*?),(.+)/
) {
my $family = $1 || 'OTHER';
$product{ $family }{ hour } += $2;
$product{ $family }{ count }++;
}
}

print Dumper \%product;


Or with a hash:

my %valid = qw(
TC_COMMUNITY TC_COMMUNITY
WEBTOOLS WEBTOOLS
SOLID_EDGE SOLID_EDGE
TC_ENGR-IMAN TC_ENGR-IMAN
UNIGRAPHICS_NX NX
NX NX
);
my %product;
while ( <DATA> ) {
my ( $family, $hour ) = split /,/;
$family = $valid{ $family } || 'OTHER';
$product{ $family }{ hour } += $hour;
$product{ $family }{ count }++;
}

print Dumper \%product;



John