CODEWITHSUNDEEP
java
user1658887
user1658887

Reputation: 459

Design pattern to reduce redundancy

I am currently designing a class which heavily makes use of reflection by manipulating the declared fields. Hence, a lot of methods have something in common in terms of their body, which is (hopefully) illustrated by this java code:

import java.lang.reflect.Field;

public class foo {
    public void foo1(/* arguments1 */) {
        for (Field f : getClass().getDeclaredFields()) {
            // do some stuff using arguments1
        }
    }

    public void foo2(/* arguments2 */) {
        for (Field f : getClass().getDeclaredFields()) {
            // do some stuff using arguments2
        }
    }

    public void foo3(/* arguments3 */) {
        for (Field f : getClass().getDeclaredFields()) {
            // do some stuff using arguments3
        }
    }

    //and so on...
}

Depending on how many methods this class will finally contain, could this be considered a design flaw? If I want to use getFields() instead of getDeclaredFields() for example, I would need to replace every occurrence of getDeclaredFields(). This does not sound like good programming practice to me. In my case this might not be a very realistic scenario, but for the sake of interest I would like to know if there is a design pattern or a concept which tackles this problem.

[EDIT]

To avoid additional misunderstandings: The operations inside the loop depend on the arguments given by foo1, foo2 etc.. and those arguments are not always the same for each method. I illustrated this fact poorly, sry. I improved the given code to demonstrate it better.

Upvotes: 4

Views: 1660

Answers (3)

Ted Hopp
Ted Hopp

Reputation: 234847

You might want to define an interface for the body of the loop:

interface FieldOperation {
    void doSomeStuff(Field f);
}

Then you can write a single looping method in place of foo1, foo2, and foo3:

public void foo(/* arguments */, FieldOperation op) {
    for (Field f : getClass().getDeclaredFields()) {
        op.doSomeStuff(f);
    }
}

You can then instantiate several FieldOperation objects:

FieldOperation foo1Operation = new FieldOperation() {
    void doSomeStuff(Field f) {
        // do some stuff that used to be in foo1()
    }
}
// etc.

This scales nicely and separates the logic of which fields to access from the operation that you want to do on each field.

EDIT If each foo* requires a different set of arguments, I'd suggest packaging them up as classes:

class Foo1Args { . . . }
class Foo2Args { . . . }
class Foo3Args { . . . }

Then you can make your interface generic:

interface FieldOperation<T> {
    void doSomeStuff(Field f, T args);
}

and define foo to be a generic method:

public <T> void foo(T args, FieldOperation<T> op) {
    for (Field f : getClass().getDeclaredFields()) {
        op.doSomeStuff(f, args);
    }
}

Upvotes: 5

datguy
datguy

Reputation: 643

Instead of creating a new field that you need to need to filter from your other methods, why not create a method to get the fields:

public class foo {
    private Field[] getDeclaredFields() { 
        return getClass().getDeclaredFields();
    }

    public void foo1(/* arguments */) {
        for (Field f : getDeclaredFields()) {
            // do some stuff
        }
    }

    public void foo2(/* arguments */) {
        for (Field f : getDeclaredFields()) {
            // do some stuff
        }
    }

    public void foo3(/* arguments */) {
        for (Field f : getDeclaredFields()) {
            // do some stuff
        }
    }
}

Upvotes: 0

meriton
meriton

Reputation: 70574

To reuse the logic to find the fields, you can externalize it to a separate method or field:

public class foo {
    private final Field[] fields = getClass().getDeclaredFields();

    public void foo1(/* arguments */) {
        for (Field f : fields) {
            // do some stuff
        }
    }

    public void foo2(/* arguments */) {
        for (Field f : fields) {
            // do some stuff
        }
    }

    public void foo3(/* arguments */) {
        for (Field f : fields) {
            // do some stuff
        }
    }
}

Upvotes: 0

Related Questions