Factor out loops

Question

I am practicing how to find and remove dead code. I have the following code:

              int c1 = Integer.parseInt(args[0]) ;
           int c2 = Integer.parseInt(args[1]) ;
           int c3 = Integer.parseInt(args[2]) ;

/*  1 */           c1 += 7 ; 
/*  2 */           System.out.println( c1 ) ; 

/*  3 */       while (c1 % 8 != 0)
/*  4 */              if ( c1 % 16 == 0 ) ; 
/*  5 */              else
/*  6 */         do 
/*  7 */                 {
/*  8 */                    c1 += 7 ; 
/*  9 */                    System.out.println( c1 ) ; 
/* 10 */                    if ( c2 < c3 )
/* 11 */                       { c1 = c1+c1 ; 
/* 12 */                         c3 ++ ; 
/* 13 */                         c1 /= 2 ; 
/* 14 */                         c3 -= 1 ; 
/* 15 */                       }
/* 16 */                 }
/* 17 */                 while ( c1 % 8 != 0 ) ;

/* 18 */           c1 += 7 ; 
/* 19 */           System.out.println( c1 ) ; 
        }     

My oppinion on this code: first the if statement can be removed, because it does not effect the execution of the rest of the code. Besides c1%16 is the same as c1%8.

How do I handle the loops?

Answer 1

I would start from the inner code of the loop: For example inside the inner if you have

c1 = c1+c1 ; 
c3 ++ ; 
c1 /= 2 ; 
c3 -= 1 ; 

the first and third line cancel each other .. and the same with the second and fourth. Removing those you get the inner if like this:

if ( c2 < c3 )
{
}

which can be eliminated (also removing the need for c2, c3 vars) thus making the enclosing statement look like this:

do 
{
  c1 += 7 ; 
  System.out.println( c1 ) ; 
}
while ( c1 % 8 != 0 );

If we go a step up and reverse the enclosing if/else we get something like this:

if ( c1 % 16 != 0 )
    do 
    {
      c1 += 7 ; 
      System.out.println( c1 ) ; 
    }
    while ( c1 % 8 != 0 );
else 
 ;

and the empty else can be removed. Now if you another step up you get:

while (c1 % 8 != 0)
  if ( c1 % 16 != 0 )
    do 
    {
      c1 += 7 ; 
      System.out.println( c1 ) ; 
    }
    while ( c1 % 8 != 0 );

An you remove the if completely since it's already checked in the while above. Now if you write the complete code you get:

c1 += 7 ; 
System.out.println( c1 ) ; 

while (c1 % 8 != 0)
  do 
  {
    c1 += 7 ; 
    System.out.println( c1 ) ; 
  }
  while ( c1 % 8 != 0 );

c1 += 7 ; 
System.out.println( c1 ) ; 

you can remove the first while and the initial add/print altogether because the first do loop will have the same semantics.

In the end you should obtain something like this:

    do {
        c1 += 7;
        System.out.println(c1);
    }
    while (c1 % 8 != 0);

    c1 += 7;
    System.out.println(c1);

And if you don't need to actually print the intermediate values you can obtain the final c1 value via simple mathematics in 1-2 steps :-).

Answer 2

c%16 is NOT the same as c%8. If c were equal to 24, the former returns 8 and the latter 0. If c were 32 they would both be 0, but if c were 40, the former again returns 8 and the latter 0.

Lines 4/5/6 are not optimal. What is really going on is if c1%16 != 0, do the do/while loop, but the way it is written is cloogy. It is written, 'do nothing if c1%16 == 0, else do the loop', using the naked ; after the if. I would make it more readable by doing something like:

bool shouldDoLoop = c1 % 16 != 0;
if (shouldDoLoop) {
   // do/while here
}