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

optimize mapper builder #3252

Merged
merged 6 commits into from
Dec 28, 2024
Merged

Conversation

mawen12
Copy link
Contributor

@mawen12 mawen12 commented Sep 26, 2024

Hi, I have found the XMLMapperBuilder contains two points can be optimized.

First: use method instead of parent variable

private void cacheElement(XNode context) {
  ...
  Class<? extends Cache> typeClass = typeAliasRegistry.resolveAlias(type);
  ...
  Class<? extends Cache> evictionClass = typeAliasRegistry.resolveAlias(eviction);
  ...
}

Its superclass is BaseBuilder which provide method resolveAlias(alias) to do this, may be can use method directly.

Second: use lambda to lazy compute

private Discriminator processDiscriminatorElement(XNode context, Class<?> resultType, List<ResultMapping> resultMappings) {
  ...
  String resultMap = caseChild.getStringAttribute("resultMap", processNestedResultMappings(caseChild, resultMappings, resultType));
  ...
}

The method getStringAttribute(value, default) return default only when value is null.
So when value is not null, default will be ignored,
By use lambda, default will be computed only when value is null.

@coveralls
Copy link

coveralls commented Sep 26, 2024

Coverage Status

coverage: 87.18% (+0.009%) from 87.171%
when pulling 85bd9ba on mawen12:optimize-mapper-builder
into 266542a on mybatis:master.

@hazendaz
Copy link
Member

hazendaz commented Dec 28, 2024

@mawen12 What benefit does the first portion of this provide? The method doesn't actually provide any other value so it could be that we should get rid of that extra method instead or we make the fields private in all regard. I'm just not seeing the value of that by itself. The others, ok with. I did fix up the test as we want to avoid star imports.

@mawen12
Copy link
Contributor Author

mawen12 commented Dec 28, 2024

@mawen12 What benefit does the first portion of this provide? The method doesn't actually provide any other value so it could be that we should get rid of that extra method instead or we make the fields private in all regard. I'm just not seeing the value of that by itself. The others, ok with. I did fix up the test as we want to avoid star imports.

I agree with you. The first portion is due to my personal coding habit.
I will undo the changes to the first portion.

@hazendaz hazendaz self-assigned this Dec 28, 2024
@hazendaz
Copy link
Member

thanks. merging this.

@hazendaz hazendaz merged commit 1a02824 into mybatis:master Dec 28, 2024
19 checks passed
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.

3 participants