Have an idea?

Visit Sawtooth Software Feedback to share your ideas on how we can improve our products.

conjoint cards picking by using least quota`

Hi i have a requirement to pick the cards by using the least quota. Here i have designed with 4 card quota, each card consists of 14 cards in it. Based on the least count each card will be picked. I have written a lengthy code it . Is there any way of  making code to reduce

Begin Unverified Perl
 my $c1a1=999;
 my $c1a2=999;
 my $c1a3=999;
 my $c1a4=999;
 my $c1a5=999;
 my $c1a6=999;
 my $c1a7=999;
 my $c1a8=999;
 my $c1a9=999;
 my $c1a10=999;
 my $c1a11=999;
 my $c1a12=999;
 my $c1a13=999;
 my $c1a14=999;
 my $c2a1=999;
 my $c2a2=999;
 my $c2a3=999;
 my $c2a4=999;
 my $c2a5=999;
 my $c2a6=999;
 my $c2a7=999;
 my $c2a8=999;
 my $c2a9=999;
 my $c2a10=999;
 my $c2a11=999;
 my $c2a12=999;
 my $c2a13=999;
 my $c2a14=999;
 my $c3a1=999;
 my $c3a2=999;
 my $c3a3=999;
 my $c3a4=999;
 my $c3a5=999;
 my $c3a6=999;
 my $c3a7=999;
 my $c3a8=999;
 my $c3a9=999;
 my $c3a10=999;
 my $c3a11=999;
 my $c3a12=999;
 my $c3a13=999;
 my $c3a14=999;
 my $c4a1=999;
 my $c4a2=999;
 my $c4a3=999;
 my $c4a4=999;
 my $c4a5=999;
 my $c4a6=999;
 my $c4a7=999;
 my $c4a8=999;
 my $c4a9=999;
 my $c4a10=999;
 my $c4a11=999;
 my $c4a12=999;
 my $c4a13=999;
 my $c4a14=999;


 my @c1a_val;
 my @c2a_val;
 my @c3a_val;
 my @c4a_val;
 $c1a_val[0][0]= $c1a1;
 $c1a_val[1][0]= $c1a2;
 $c1a_val[2][0]= $c1a3;
 $c1a_val[3][0]= $c1a4;
 $c1a_val[4][0]= $c1a5;
 $c1a_val[5][0]= $c1a6;
 $c1a_val[6][0]= $c1a7;
 $c1a_val[7][0]= $c1a8;
 $c1a_val[8][0]= $c1a9;
 $c1a_val[9][0]= $c1a10;
 $c1a_val[10][0]= $c1a11;
 $c1a_val[11][0]= $c1a12;
 $c1a_val[12][0]= $c1a13;
 $c1a_val[13][0]= $c1a14;
 $c2a_val[0][0]= $c2a1;
 $c2a_val[1][0]= $c2a2;
 $c2a_val[2][0]= $c2a3;
 $c2a_val[3][0]= $c2a4;
 $c2a_val[4][0]= $c2a5;
 $c2a_val[5][0]= $c2a6;
 $c2a_val[6][0]= $c2a7;
 $c2a_val[7][0]= $c2a8;
 $c2a_val[8][0]= $c2a9;
 $c2a_val[9][0]= $c2a10;
 $c2a_val[10][0]= $c2a11;
 $c2a_val[11][0]= $c2a12;
 $c2a_val[12][0]= $c2a13;
 $c2a_val[13][0]= $c2a14;
 $c3a_val[0][0]= $c3a1;
 $c3a_val[1][0]= $c3a2;
 $c3a_val[2][0]= $c3a3;
 $c3a_val[3][0]= $c3a4;
 $c3a_val[4][0]= $c3a5;
 $c3a_val[5][0]= $c3a6;
 $c3a_val[6][0]= $c3a7;
 $c3a_val[7][0]= $c3a8;
 $c3a_val[8][0]= $c3a9;
 $c3a_val[9][0]= $c3a10;
 $c3a_val[10][0]= $c3a11;
 $c3a_val[11][0]= $c3a12;
 $c3a_val[12][0]= $c3a13;
 $c3a_val[13][0]= $c3a14;
 $c4a_val[0][0]= $c4a1;
 $c4a_val[1][0]= $c4a2;
 $c4a_val[2][0]= $c4a3;
 $c4a_val[3][0]= $c4a4;
 $c4a_val[4][0]= $c4a5;
 $c4a_val[5][0]= $c4a6;
 $c4a_val[6][0]= $c4a7;
 $c4a_val[7][0]= $c4a8;
 $c4a_val[8][0]= $c4a9;
 $c4a_val[9][0]= $c4a10;
 $c4a_val[10][0]= $c4a11;
 $c4a_val[11][0]= $c4a12;
 $c4a_val[12][0]= $c4a13;
 $c4a_val[13][0]= $c4a14;
 my @c1a_min;
  my @c2a_min;

  my @c3a_min;

  my @c4a_min;

 my $x1;
 my $x2;
 my $x3;
 for (my $i=0;$i<=3;$i++)
     if($c1a_min[0][1]==$c2a_min[$i][1] ||$x1==$c2a_min[$i][1])

 for(my $i=0;$i<=4;$i++)
     if($c1a_min[0][1]==$c4a_min[$i][1] ||$x1==$c4a_min[$i][1] ||$x2==$c4a_min[$i][1])


End Unverified
asked Mar 2, 2017 by sandeepkapalawai Bronze (1,440 points)
retagged Mar 2, 2017 by Walter Williams

1 Answer

0 votes
I'm glad you asked.  Code reviews like this can be a great way to learn new coding tricks.

The first thing that jumps out to me is that you have a number of "middle man" lines of code that you can cut out.  For example, these three lines appear in your code:

my $c1a1 = 999;
$c1a1 = QUOTACELLCOMPLETES("CardQuota1", 1);
$c1a_val[0][0] = $c1a1;

You can remove the first two lines and just use this in place of the third line:

$c1a_val[0][0] = QUOTACELLCOMPLETES("CardQuota1", 1);

Second, you've got a lot of code that could be more easily done with a loop.  Here's lines 144-157 done with a for loop:

for (my $i = 0; $i <= 13; $i++) {
    $c1a_val[$i][1] = $i + 1;

Combining those two improvements, you can get the entirety of defining @c1a_val down to five lines of code:

my @c1a_val;
for (my $i = 0; $i <= 13; $i++) {
    $c1a_val[$i][0] = QUOTACELLCOMPLETES('CardQuota1'. $i + 1);
    $c1a_val[$i][1] = $i + 1;

Doing that for c1a, c2a, c3a, and c4a should be a huge code reduction.  Beyond that, I could just offer a few miscellaneous tips.

The sortings can be done with one line instead of two:

my @c1a_min = sort{$a->[0]<=>$b->[0]} @c1a_val;

x1 could be defined with a one-line ternary operator rather than with the if-else:

my $x1 = ($c1a_min[0][1] == $c2a_min[0][1]) ? $c2a_min[1][1] : $c2a_min[0][1];

Instead of having an empty if then a non-empty else, you can rework the code to just need the if.  Recall that "not (A or B or C)" is equivalent to "(not A) and (not B) and (not C)."

if ($c1a_min[0][1] != $c2a_min[$i][1] && $x1 != $c2a_min[$i][1]) {
    $x2 = $c2a_min[$i][1];
    $i = 4;

if ($c1a_min[0][1] != $c4a_min[$i][1] && $x1 != $c4a_min[$i][1] && $x2 != $c4a_min[$i][1]) {
    $x3 = $c4a_min[$i][1];
    $i = 4;

Finally, lines 288 and 300 could be replaced with this line of code to immediately end the for loop:

answered Mar 2, 2017 by Zachary Platinum Sawtooth Software, Inc. (72,325 points)