Zequan Wu February 2016

How to avoid the repeated code in java?

I'm just beginner in programming. uf is a union-find class with the method union which connects the roots of two node. This piece of code is responsible for opening a site of a grid and union the site with its neighbor if any of neighbor is opened. And if one of its neighbors is full, then fill all nodes that connected with the site. This is the actual code:

        if(i == 1){
            uf.union(len*len, xyTo1D(i,j));
            if(existAndOpen(i+1,j)){
                uf2.union(xyTo1D(i+1,j), xyTo1D(i,j));
                uf.union(xyTo1D(i,j), xyTo1D(i+1,j));
            }
            if(existAndOpen(i-1,j)){
                uf2.union(xyTo1D(i-1,j), xyTo1D(i,j));
                uf.union(xyTo1D(i,j), xyTo1D(i-1,j));
            }
            if(existAndOpen(i,j-1)){
                uf2.union(xyTo1D(i,j-1), xyTo1D(i,j));
                uf.union(xyTo1D(i,j), xyTo1D(i,j-1));
            }
            if(!(j == len && i == len)){
                if(existAndOpen(i,j+1)){
                    uf2.union(xyTo1D(i,j+1), xyTo1D(i,j));
                    uf.union(xyTo1D(i,j), xyTo1D(i,j+1));
                }
            }
        }
        else{
        if(existAndFull(i+1,j)){
            uf2.union(xyTo1D(i+1,j), xyTo1D(i,j));
            uf.union(xyTo1D(i,j), xyTo1D(i+1,j));
        }
        if(existAndFull(i-1,j)){
            uf2.union(xyTo1D(i-1,j), xyTo1D(i,j));
            uf.union(xyTo1D(i,j), xyTo1D(i-1,j));
        }
        if(existAndFull(i,j-1)){
            uf2.union(xyTo1D(i,j-1), xyTo1D(i,j));
            uf.union(xyTo1D(i,j), xyTo1D(i,j-1));
        }
        if(!(j== len && i == len)){
            if(existAndFull(i,j+1)){
                uf2.union(xyTo1D(i,j+1), xyTo1D(i,j));
                uf.union(xyTo1D(i,j), xyTo1D(i,j+1));
            }
        }
        if(existAndOpen(i+1,j)){
            uf.union(xyTo1D(i,j), xyTo1D(i+1,j));
        }
        if(existAndOpen(i-1,j)){
            uf.union(xyTo1D(i,j), xyTo1D(        

Answers


saka1029 February 2016

Try this

boolean f1(int a, int b) { }
boolean f2(int a, int b) { }
void A(int a, int b) { }

void testAndA(BiPredicate<Integer, Integer> p, int a, int b) {
    if (p.test(a, b))
        A(a, b);
}

and

    if(x == 1){
        testAndA(this::f1, x + 1, y);
        testAndA(this::f1, x, y + 1);
    } else {
        testAndA(this::f2, x + 1, y);
        testAndA(this::f2, x, y + 1);
    }


Spencer February 2016

You can write a loop to loop through all of the different values that could be passed to f1(), such as something like:

for (int deltax = -1; deltax <= 1; deltax++) {
    for (int deltay = -1; deltay <= 1; deltay++) {
        if (f1(x + deltax, y + deltay)) {
            A(x + deltax, y + deltay);
        }
    }
}

Of course change the start and end values of deltax and deltay depending on which conditions you need to check.


cybersam February 2016

You say there are "more if statements in each block." Reading between the lines, I assume that means you need to make more calls to f1/f2 and A, but with different x and y offsets.

Here is a program that shows a way to refactor the code to avoid coding repetition. Its main features are:

  • It uses object orientation to abstract the inner if blocks.
  • It uses an offsets array to represent the x/y offsets for each inner if block.
  • The doIt() method uses a loop to invoke the inner if blocks.

    public class Main {
    
        static interface F {
          void f(int i, int j);
        }
    
        static class F1Caller implements F {
          public void f(int a, int b) {
            if (f1(a, b)) {
              A(a, b);
            }
          }
        }
    
        static class F2Caller implements F {
          public void f(int a, int b) {
            if (f2(a, b)) {
              A(a, b);
            }
          }
        }
    
        static boolean f1(int a, int b) { System.out.print("  f1. "); return true; }
        static boolean f2(int a, int b) { System.out.print("  f2. "); return true; }
        static void A(int a, int b) { System.out.println("a: " + a + ", b: " + b); }
    
        static F1Caller f1Caller = new F1Caller();
        static F2Caller f2Caller = new F2Caller();
    
        // x and y offsets for each call to f1/f2.
        // Add more offset rows, as needed.
        static int offsets[][] = {
            {1,  0},
            {0, -1}
        };
    
        static void doIt(int x, int y) {
          System.out.println("x: " + x + ", y: " + y);
    
          F f = (x == 1) ? f1Caller : f2Caller;
          for (int k = 0; k < offsets.length; k++) {
              f.f(x + offsets[k][0], y + offsets[k][1]);
          }
        }
    
    
        public static void main(String[] args) {
            doIt(0, 0);
            doIt(1, 0);
        }
    }
    

The output of the above program is:


              

Anthony Law February 2016

You can combine the function together.

Using a switch.

public static boolean func(int a, int b, int fun)
{
     boolean output = false;
     switch(fun)
     {
     case 1:
         //do stuff
         output = true;
         break;
     case 2:
         //do stuff
         output = true;
         break;
     default:
         //unknown function handling
        output = false;
     }
     return output;
}

Integrate it with a for-loop:

public static void main(String[] args)
{
     int i;
     //Change this to whatever you want or set it to a argument.
     int repeat = 2;
     for(i = 1; i <= repeat; i++)
     {
          func(a, b, i);
     }
}

P.S. I recently noticed your edit, I am reading that code and giving another code to you as soon as possible.

Post Status

Asked in February 2016
Viewed 3,196 times
Voted 7
Answered 4 times

Search




Leave an answer