0

I have two classes.

public class Shape1 extends javafx.scene.shape.Path {

    public Shape1(PathElement... elements) {
        super(elements);
    }    
}

public class Shape2 extends javafx.scene.shape.Path {

    public Shape2(PathElement... elements) {
        super(elements);
    }    
}

They are not the same. They have different fields and methods, but I did not mention them for simplicity.

I create two arrays for each type of object and I only need to use one method to create them.

ArrayList<Shape1> first_shapes = create_array(50, 50, 100, 100, Class.forName("example3.Shape1"));
ArrayList<Shape2> second_shapes = create_array(50, 100, 100, 150, Class.forName("example3.Shape2"));

public static ArrayList create_array(int X1, int Y1, int X2, int Y2, Class my_class) {
    var shapes = new ArrayList<>();
    shapes.add(my_class.getConstructor(PathElement[].class).newInstance(new MoveTo(X1, Y1), new LineTo(X2, Y2)));
    ...
    return shapes;
}

The code has two problems.

  1. The class definition is inappropriate because it will not work when you change the class name or the package name and it has to be changed manually.

  2. Actually, it doesn't work at all because it crashes when you add an object to an array.

Strange is the fact that in the form above it will display an error.

wrong number of arguments

But if I use only one parameter,

shapes.add(my_class.getConstructor(PathElement[].class).newInstance(new MoveTo(X1, Y1)));

it will display an error.

argument type mismatch

It's the only solution to my problem. Otherwise I would have to change the whole algorithm.

Please help

Thank you

Michal Rama
  • 173
  • 1
  • 1
  • 10
  • 1
    The renaming issue should be not an issue at all unless you're using `Class.forName("SomeType")` instead of `SomeType.class`. The latter case should be treated by the renaming functionality of your IDE. Furthermore I'd recommend using a `Function` instead: this is more flexible, does allow the compiler to check the existance of the appropriate method and does not require to be allowed to reflectively access the containing class. Furthermore it would also work for non-constructor ways of creating the path: `create_array(X1, Y1, X2, Y2, Shape1::new);` – fabian Nov 15 '19 at 06:55

2 Answers2

3

First, enable all compiler warnings, and pay attention to them. You never want to use Class or ArrayList without a generic type; see What is a raw type and why shouldn't we use it?.

The best policy regarding reflection is to avoid using reflection.

  • Reflection is slow. It is unlikely to be optimized by the just-in-time compiler.
  • Reflection is unsafe. The compiler cannot check whether you’ve made a mistake, such as a spelling error or a method signature error.
  • Reflection is hard to read. You will always need to be able to understand and maintain your code at some later date. And in the professional world, other people will need to maintain your code, probably long after you’re gone.

Instead of using reflection, treat each constructor as a function. Specifically, a function which takes a sequence of PathElements as input, and produces a new instance of a particular class as output. In Java, that is written as:

Function<PathElement[], S>

where S is itself defined as <S extends Path>, that is, a known type which is inherits from Path.

You can pass this known type and known constructor to your method:

public static <S extends Path> ArrayList<S> create_array(
    int X1, int Y1, int X2, int Y2,
    Function<PathElement[], S> shapeConstructor) {

    var shapes = new ArrayList<S>();
    shapes.add(shapeConstructor.apply(new PathElement[] {
        new MoveTo(X1, Y1), new LineTo(X2, Y2)
    }));
    //...
    return shapes;
}

Now you have code that is both type safe and capable of catching any errors in spelling or syntax at compile time.

Invocations of this method would look like this:

ArrayList<Shape1> shapeList1 = create_array(10, 10, 50, 50, Shape1::new);
ArrayList<Shape2> shapeList2 = create_array(20, 20, 80, 80, Shape2::new);
VGR
  • 40,506
  • 4
  • 48
  • 63
0
  1. problem: Use Shape1.class instead of Class.forName("example3.Shape1"). This way, changing the pkg and class name with standard renaming refactoring changes it everywhere. Class.forName() is needed for Class resolution which you cannot predict, it's dynamic, f.e. input from user. You need only the static Shape1.class reference to the Class<Shape1> instance.

Also, change the method parameter to accept generic Class<T> myClass and return a List of the same type T like this:

public static <T> ArrayList<T> create_array(int X1, int Y1, int X2, int Y2, Class<T> my_class) { ... }

This way, it is type safe with generics, without warnings about raw types.

  1. Maybe if it expects an array argument, you should wrap the single parameter in an array.

wrong number of arguments means you are calling the constructor expecting a single array argument with actually 2 arguments. Varargs are not wrapped in the array automatically with reflection, obviously.

argument type mismatch: again, it expects an array PathElement[], but you provide only PathElement


Alternative solution

Reflection is most of the time the last resort, for various reasons. Is using reflection really necessary in this case? You could solve this problem by inheritance. Sure, both of them already extend a superclass & have different fields and methods, but you can create an almost empty intermediate abstract class BaseShape which will have your method in it (except the Class parameter ofc) and the used constructor - it will be inherited.

From OOP standpoint, this is definitely a cleaner solution, than a static method using reflection.

Hawk
  • 2,042
  • 8
  • 16