Skip to content

Conversation

LazyRichard
Copy link
Contributor

Hi.

First of all, I don't speak English very well, so if my wording is inappropriate, please kindly correct me.

I want to contribute a collection to an API that takes inputs.

If I try to run the code below first, it will fail, because the compiler will complain like Cannot resolve method 'select(List<SqlColumn<?>>)'

package org.example;

import org.mybatis.dynamic.sql.AliasableSqlTable;
import org.mybatis.dynamic.sql.SqlColumn;
import org.mybatis.dynamic.sql.render.RenderingStrategies;

import java.sql.JDBCType;
import java.util.Arrays;
import java.util.List;

import static org.mybatis.dynamic.sql.SqlBuilder.*;
import static org.example.Main.UserTableDynamicSupport.user;

public class Main {

    public static final class UserTableDynamicSupport {

        public static UserTable user = new UserTable();
        public static final class UserTable extends AliasableSqlTable<UserTable> {

            public final SqlColumn<Long> id = column("id", JDBCType.INTEGER);
            public final SqlColumn<String> name = column("name", JDBCType.VARCHAR);

            public UserTable() {
                super("User", UserTable::new);
            }
        }
    }

    public static List<SqlColumn<?>> columns = Arrays.asList(
            user.id,
            user.name
    );

    public static void main(String[] args) {
        var statement = select(columns).from(user).build().render(RenderingStrategies.MYBATIS3);
    }
}

To fix this, I modified the type of the API that takes collections as input to the following.

public static QueryExpressionDSL.FromGatherer<SelectModel> select(Collection<? extends BasicColumn> selectList)

This allows us to accept a variety of child types.

Of course, I could have changed it to BasicColumn via typecasting, but this is inconvenient in certain situations.

moreover this is also more sophisticated I think

Thanks

@coveralls
Copy link

Coverage Status

Coverage: 100.0%. Remained the same when pulling 4be9c77 on LazyRichard:feat/bounded-type-params into 3d8b51a on mybatis:master.

@jeffgbutler
Copy link
Member

Thanks for contributing! This is a good idea but it is not needed in all the places you changed. It looks like you changed virtually every method that takes a collection or list.

Classes like AndOrCriteriaGroup and JoinCriterion don't have any subclasses and are not generic so I don't think we need this change on methods that take collections of those classes.

Several methods already accept List<SqlColumn<?>> as a parameter - we don't need this change on those methods either.

I think this change makes sense for methods that accept collections of BasicColumn, SortSpecification, and AbstractColumnMapping.

Please update your branch by removing the changes to methods that aren't needed, then I'll be happy to merge this.

@LazyRichard LazyRichard force-pushed the feat/bounded-type-params branch from 4be9c77 to 4f5b7a0 Compare April 28, 2023 14:42
@jeffgbutler jeffgbutler added this to the 1.5.1 milestone Apr 28, 2023
@jeffgbutler jeffgbutler merged commit 6dd4e90 into mybatis:master Apr 28, 2023
@jeffgbutler
Copy link
Member

Thanks!

@LazyRichard LazyRichard deleted the feat/bounded-type-params branch April 28, 2023 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants