Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: use generics #1561

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Runrioter
Copy link

No description provided.

@simonetripodi
Copy link
Member

LGTM 👍

@kazuki43zoo
Copy link
Member

@Runrioter @simonetripodi

What benefit does this change have for MyBatis users? I know If applied this change in existing implementation class, compile warning(unchecked warning) occur. I thinks have not benefit for user by only this change. Also at least, I think should wait to apply this change until next minor or major version up(Probably, we should be consider to modify the Plugin#wrap method together too).

WDYT?

@Runrioter
Copy link
Author

@kazuki43zoo

What benefit does this change have for MyBatis users?

No. Just refactor this for coding easily without unnecessary casting

compile warning(unchecked warning) occur

Can you paste compile warning here ?

@kazuki43zoo
Copy link
Member

Can you paste compile warning here ?

image

  • The above source code (inner class that use in MyBatis's test case)
  @Intercepts({
      @Signature(type = Map.class, method = "get", args = {Object.class})})
  public static class AlwaysMapPlugin implements Interceptor {
    @Override
    public Object intercept(Invocation invocation) {
      return "Always";
    }

    @Override
    public Object plugin(Object target) {
      return Plugin.wrap(target, this);
    }

    @Override
    public void setProperties(Properties properties) {
    }
  }

For prevent above waring, developer need to change existing source code as follow:

  • Add @SuppressWaring("unchecked") on method level
    @Override
    @SuppressWarnings("unchecked")
    public Object plugin(Object target) {
      return Plugin.wrap(target, this);
    }
  • Change method signature and adding cast that return value of Plugin#wrap (And need to add the @SuppressWaring("unchecked") for unchecked cast...)
    @Override
    @SuppressWarnings("unchecked")
    public <T> T plugin(T target) {
      return (T) Plugin.wrap(target, this);
    }

Libraries code will polishing but the user's point of view, it seems that there is no benefit to applying this change (in my opinion).

@Runrioter
Copy link
Author

Thanks for your clarify. This is a little break when we override this method. I will modify Plugin#wrap too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants