0
\$\begingroup\$

So i am working on a project and my question is as follows. In the code below is alot of code duplication going. Is it possible to avoid code duplication in such cases? What would be the best practices to avoid the code duplication below. I hope someone can point me to the right direction.

public void rechts() {
        openBarricade(veld, 1, 0);
        int x = locatie[0] / 50;
        int y = locatie[1] / 50;
        if (x < 9 && veld.getVak(x + 1, y).isLoopbaar()) {
            switch (frame) {
                case 0:
                    setImage(new ImageIcon(getClass().getResource("/images/speler/speler_r1.png")).getImage());
                    frame++;
                    break;
                case 1:
                    setImage(new ImageIcon(getClass().getResource("/images/speler/speler_r2.png")).getImage());
                    frame++;
                    break;
                case 2:
                    setImage(new ImageIcon(getClass().getResource("/images/speler/speler_r3.png")).getImage());
                    frame++;
                    break;
                default:
                    frame = 0;
                    setImage(new ImageIcon(getClass().getResource("/images/speler/speler_r2.png")).getImage());
                    break;
            }
            lopen(UI.OBJECTBREEDTE, 0);
        }

    }

    public void links() {
        openBarricade(veld, -1, 0);
        int x = locatie[0] / 50;
        int y = locatie[1] / 50;

        if (x > 0 && veld.getVak(x - 1, y).isLoopbaar()) {
            switch (frame) {
                case 0:
                    setImage(new ImageIcon(getClass().getResource("/images/speler/speler_l1.png")).getImage());
                    frame++;
                    break;
                case 1:
                    setImage(new ImageIcon(getClass().getResource("/images/speler/speler_l2.png")).getImage());
                    frame++;
                    break;
                case 2:
                    setImage(new ImageIcon(getClass().getResource("/images/speler/speler_l3.png")).getImage());
                    frame++;
                    break;
                default:
                    frame = 0;
                    setImage(new ImageIcon(getClass().getResource("/images/speler/speler_l2.png")).getImage());
                    break;
            }
            lopen(-UI.OBJECTBREEDTE, 0);

        }

    }

   public void omHoog() {
        openBarricade(veld, 0, -1);
        int x = locatie[0] / 50;
        int y = locatie[1] / 50;

        if (y > 0 && veld.getVak(x, y - 1).isLoopbaar()) {
            switch (frame) {
                case 0:
                    setImage(new ImageIcon(getClass().getResource("/images/speler/speler_u1.png")).getImage());
                    frame++;
                    break;
                case 1:
                    setImage(new ImageIcon(getClass().getResource("/images/speler/speler_u2.png")).getImage());
                    frame++;
                    break;
                case 2:
                    setImage(new ImageIcon(getClass().getResource("/images/speler/speler_u3.png")).getImage());
                    frame++;
                    break;
                default:
                    frame = 0;
                    setImage(new ImageIcon(getClass().getResource("/images/speler/speler_u2.png")).getImage());
                    break;
            }
            lopen(0, -UI.OBJECTBREEDTE);

        }
    }

    public void omLaag() {
        openBarricade(veld, 0, 1);
        int x = locatie[0] / 50;
        int y = locatie[1] / 50;

        if (y < 9 && veld.getVak(x, y + 1).isLoopbaar()) {
            switch (frame) {
                case 0:
                    setImage(new ImageIcon(getClass().getResource("/images/speler/speler_d1.png")).getImage());
                    frame++;
                    break;
                case 1:
                    setImage(new ImageIcon(getClass().getResource("/images/speler/speler_d2.png")).getImage());
                    frame++;
                    break;
                case 2:
                    setImage(new ImageIcon(getClass().getResource("/images/speler/speler_d3.png")).getImage());
                    frame++;
                    break;
                default:
                    frame = 0;
                    setImage(new ImageIcon(getClass().getResource("/images/speler/speler_d2.png")).getImage());
                    break;
            }

            lopen(0, UI.OBJECTBREEDTE);

        }
    }
\$\endgroup\$

closed as unclear what you're asking by Jamal Apr 28 '18 at 18:48

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 1
    \$\begingroup\$ The current question title, which states your concerns about the code, is too general to be useful here. The site standard is for the title to simply state the task accomplished by the code. Please see How to Ask for examples, and revise the title accordingly. \$\endgroup\$ – Jamal Apr 15 '18 at 18:17
3
\$\begingroup\$

here is what I did: I focused on the switch statements since it seems they are almost identical. it turns out they differ in one letter in the resource name. So it makes sense to create a method that recevies this letter. There is also duplication in the case clauses, so I replaced the switch with an if that separates default clause from the rest:

public void commonSetImage(String spelerLetter) {
    // default values
    int resourceNum = 2;
    String resource = String.format("/images/speler/speler_%s%d.png", spelerLetter, resourceNum);
    // in case frame is between 0 to 2, set resource num according to frame
    if (frame >= 0 && frame <= 2) {
        resourceNum = frame + 1;
        resource = String.format("/images/speler/speler_%s%d.png", spelerLetter, resourceNum);
        setImage(new ImageIcon(getClass().getResource(resource)).getImage());
        frame++;
    } else {
        frame = 0;
        setImage(new ImageIcon(getClass().getResource(resource)).getImage());
    }
}

now the original methods can make use of the new method:

public void rechts() {
    openBarricade(veld, 1, 0);
    int x = locatie[0] / 50;
    int y = locatie[1] / 50;
    if (x < 9 && veld.getVak(x + 1, y).isLoopbaar()) {
        commonSetImage("r");
        lopen(UI.OBJECTBREEDTE, 0);
    }
}
\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.