querydsl / querydsl

Unified Queries for Java
https://querydsl.com
Apache License 2.0
4.74k stars 873 forks source link

Polymorphic CaseBuilder.Initial.then #702

Closed dmitrygusev closed 10 years ago

dmitrygusev commented 10 years ago

Using variable of general type (like Expression<T>) in new CaseBuilder().when().then(HERE) will always call version of then with this signature:

public <A> Cases<A, SimpleExpression<A>> then(Expression<A> expr)

which means you will get SimpleOperation in response and you can't use it in "order by" clauses and other methods that require concrete type because it can't be cast from SimpleOperation.

This could be solved by providing polymorphic version of "then" that will decide which overload to invoke at runtime based on type of expression, here's an example of how this method may look like:

class CaseBuilder {
    // ...
    class Initial {
        // ...
        public <A,Q extends Expression<A>> Cases<A,Q> then2(Expression<A> expr) {
            if (expr instanceof BooleanExpression) {
                return (Cases<A, Q>) then((BooleanExpression) expr);
            }
            if (expr instanceof NumberExpression) {
                return then((NumberExpression) expr);
            }
            if (expr instanceof DateExpression) {
                return then((DateExpression) expr);
            }
            if (expr instanceof DateTimeExpression) {
                return then((DateTimeExpression) expr);
            }
            if (expr instanceof StringExpression) {
                return (Cases<A, Q>) then((StringExpression) expr);
            }
            if (expr instanceof TimeExpression) {
                return then((TimeExpression) expr);
            }
            return (Cases<A, Q>) then(expr);
        }
timowest commented 10 years ago

@dmitrygusev Could you comment if this change is sufficient https://github.com/mysema/querydsl/pull/703 ?

The then2 suggestion has the problem that you will need to provide the Q type argument to the call since it can't be derived from the arguments.

dmitrygusev commented 10 years ago

No, this doesn't help much. Here's what I did: I have a method that builds case expression based on some predefined condition set, but "then"-part of that case expression may differ:

    interface ReminderExpression<T> {
        Expression<T> then(QReminder reminder);
    }

    QReminder reminder1 = new QReminder("reminder1");
    QReminder reminder2 = new QReminder("reminder2");
    QReminder reminder3 = new QReminder("reminder3");

    <T> Expression<T> getReminderCase(ReminderExpression<T> thenExpr) {
        return new BetterCaseBuilder()
            .when(CONDITION1)
            .then2(thenExpr.then(reminder1))
            .when(CONDITION2)
            .then(thenExpr.then(reminder2))
            .otherwise(thenExpr.then(reminder3));
    }

Then I have few another methods that use this method to build actual expression, like:

     DateExpression<Date> getReminderDateCase() {
        return (DateExpression<Date>) getReminderCase(new ReminderExpression<Date>() {
            @Override public Expression<Date> then(QReminder reminder) {
                return dateAdd(invoice.dueOn, reminder.offsetInDays);
            }
        });
    }

As you mentioned, then2 implementation can't derive Q type from arguments, that's why I have to cast result to desired type, and it works fine for me here.

I tried solution from #703 and this example worked, but I have another method that builds actual expression and returns NumberExpression from NumberPath, and it doesn't compile:

    interface ReminderExpression<T extends Comparable<T>> {
        ComparableExpression<T> then(QReminder reminder);
    }

    NumberExpression<Byte> getReminderOrderNumberCase() {
        return (NumberExpression<Byte>) getReminderCase(new ReminderExpression<Byte>() {
            @Override public ComparableExpression<Byte> then(QReminder reminder) {
                // COMPILE ERROR:
                // Type mismatch: cannot convert
                // from NumberPath<Byte> to ComparableExpression<Byte>
                return reminder.orderNumber;
            }
        });
    }
timowest commented 10 years ago

@dmitrygusev I used now your then2 code in the pull request. Could you take a look?

It should work with a single method, since all the other Expression types are subclasses of SimpleExpression.

dmitrygusev commented 10 years ago

yep, works good now, thanks

timowest commented 10 years ago

Ok, good. Are these replacements also ok for you?

-  public Cases<java.sql.Date, DateExpression<java.sql.Date>> thenDate(java.sql.Date date)
+  public Cases<java.sql.Date, DateExpression<java.sql.Date>> then(java.sql.Date date) 
-  public Cases<Timestamp, DateTimeExpression<Timestamp>> thenDateTime(Timestamp ts)
+  public Cases<Timestamp, DateTimeExpression<Timestamp>> then(Timestamp ts)
-  public Cases<java.util.Date, DateTimeExpression<java.util.Date>> thenDateTime(java.util.Date date)
+  public Cases<java.util.Date, DateTimeExpression<java.util.Date>> then(java.util.Date date)
dmitrygusev commented 10 years ago

For me, yes. All my tests passed with changes from #703.

timowest commented 10 years ago

Released in 3.3.3