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

Simplify getCache() method in CaffeineCacheManager #24376

Closed
wants to merge 1 commit into from
Closed

Simplify getCache() method in CaffeineCacheManager #24376

wants to merge 1 commit into from

Conversation

bananayong
Copy link
Contributor

I would like to make some contributions.

Current spring framework is based on JDK8. So, CaffeineCacheManager.getCache() method can be simplified with Map.computeIfAbsent

Thank you.

Signed-off-by: Kwangyong Kim <banana.yong@gmail.com>
@pivotal-issuemaster
Copy link

@bananayong Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@bananayong Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 16, 2020
@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 16, 2020
@sbrannen sbrannen self-assigned this Jan 16, 2020
@sbrannen sbrannen added this to the 5.2.4 milestone Jan 16, 2020
@sbrannen sbrannen closed this in 16e49bf Jan 16, 2020
@sbrannen
Copy link
Member

Thanks for the PR!

I ended up implementing this slightly differently in 16e49bf.

@bananayong
Copy link
Contributor Author

Thank you to reflect my contribution.
It looks simpler!

But, I would like to give an opinion.

cacheName -> this.dynamic ? createCaffeineCache(cacheName) : null

This lambda expression accesses this.dynamic member field. It causes capturing variable.
getCache method will create new lambda instance whenever called.
It can be avoided if this.dynamic is checked outside lambda.

Please, ignore this comment if there is no big benefits.
Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants