coin-or / jorlib

Java Operations Research Library
GNU Lesser General Public License v2.1
62 stars 37 forks source link

Reflection may fail in constructing pricing problem solver #26

Open rowtricker opened 7 years ago

rowtricker commented 7 years ago

Currently, reflection is used to create a new instance of a PricingProblemSolver extends AbstractPricingProblemSolver.

However, reflection may fail in case a constructor is used that uses additional parameters over those used in the default constructor. Consider for example

PricingProblemSolver extends AbstractPricingProblemSolver<T,U,V> {
   public PricingProblemSolver(T dataModel, V pricingProblem, boolean flag) {
      ...
   }

   ...
}

For this example, reflection will fail. Moreover, it is relatively difficult to track down the actual cause of this.

I would suggest to change the use of reflection here by letting the user provide a factory himself for creating new instances of the solver. The actual code needed for this would be relatively limited, especially with the use of lambda's.

jkinable commented 7 years ago

I've never tried this myself. This reflection stuff was quite complex. In what case would you want to change this behavior? Can't use just create the PricingProblemSolver and set your boolean flag through some method in the solver (instead of setting this in the constructor)?

rowtricker commented 7 years ago

I have actually had this happening to me a couple of times when I wanted to add an extra constructor parameter. And I heard that another user of the library was also struggling with this at some moment. What is also difficult about this issue is that the standard error message is quite uninformative, making it difficult to find the actual bug. Especially, as one needs to know some details of reflection to see the issue.

A simple setter would be a solution to the actual bug, but is not really a solution for the underlying problem that the user is likely unaware of the fact that he can't add additional parameters. Clearly, that this is impossible makes sense from the framework perspective, but might not make sense to the user.

There are probably some possible solutions, but some might require some extra work by the user. One solution would be to let the user pass a solver factory instead, so let the user provide a class that implements PricingProblemSolverFactory. In that case, no reflection would be needed at all, thereby also preventing all the possible issues with reflection.