JSqlParser icon indicating copy to clipboard operation
JSqlParser copied to clipboard

Add marker interfaces to allow OOP classification of expressions

Open thomaszbz opened this issue 8 years ago • 2 comments

This is an enhancement against JsqlParser version 1.0.0:

For predicate extraction, I have a use case which needs to distinguish between all sorts of BinaryExpressions (I only want relational operators).

By package name, JSqlParser already knows if an operator is relational. So I can test with something like

    if (!"net.sf.jsqlparser.expression.operators.relational"
            .equals(binaryExpression.getClass().getPackage().getName())
            ) {
        return;
    }

However, that implies an expensive string comparison. Plus, can't IDE's easily refactor these strings.

I'd rather use something like instanceof, but want to let the object model do all the classification (and avoid to write down all cases with if statements).

For an OOP representation of the classification, I suggest that JSqlParser either

  • uses an additional layer of parent class / sub class between BinaryExpression and its subclasses OR
  • tags the subclasses via marker interface (which is an empty interface just for tagging and testing)

I think that marker interfaces would

  • keep the object model flat
  • be minimal-invasive
  • be flexible in that it allows to add additional dimensions of classification (not only in respect to relational/arithmetic/etc.)
  • allow to add further levels where needed (by extending marker interfaces)

So far, I'd go for marker interfaces.

On the other side: Using class inheritance, one is forced to decide for which dimension it should be. That also implies a breaking change if that changes over time. That could easily happen if, later on, code redundancy is to be removed technically via class inheritance whereas it is questionable if that goes along with the dimension I'm interested in.

Third option would be to use both, marker interfaces and class inheritance. With that in mind, the one or the other could still be added later without a breaking change. Which brings me to the conclusion that, in the long run, starting with marker interfaces should be painless later on.

Conclusion: Subclasses of BinaryExpression should have marker interfaces.

thomaszbz avatar Jun 26 '17 02:06 thomaszbz

So far I agree. Tagging or marking seems less invasive. I also recognized that in those packages are classes, that are not fitting your improvement suggestion: e.g. ExpressionList, MultiExpressionList, ....

Therefore this task should include moving / repackaging those classes to a better fitting package.

wumpz avatar Jun 26 '17 07:06 wumpz

moving / repackaging would put that at a 2.0 (non-passive).

AnEmortalKid avatar Oct 06 '17 02:10 AnEmortalKid