Skip to content

Relax MyBatisPagingItemReader to allow constructor instanciation #331

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

Merged
merged 3 commits into from
Apr 3, 2019
Merged

Relax MyBatisPagingItemReader to allow constructor instanciation #331

merged 3 commits into from
Apr 3, 2019

Conversation

MadJlzz
Copy link

@MadJlzz MadJlzz commented Oct 29, 2018

This PR is linked to the issue #323

Basically, I used the SqlSession interface instead of the SqlSessionTemplate implementation.

I override doOpen() and doClose() to avoid initialization of the connection through afterPropertiesSet.

This is not a final version ready to be merged since it is missing unit tests. I just wanted to show any other solution.

@kazuki43zoo
Copy link
Member

Basically, I used the SqlSession interface instead of the SqlSessionTemplate implementation.

Why? Is this changes related to this issue?
I think should be create a new issue because this change has not related with this issue.

WDYT?

@MadJlzz
Copy link
Author

MadJlzz commented Oct 30, 2018

Basically, I used the SqlSession interface instead of the SqlSessionTemplate implementation.

You are totally right, about my change, it is not necessary here. I will use SqlSessionTemplate like before. Using interface instead of implementation is the key for good encapsulation and for loose coupling in the code. That's why I did it.

@MadJlzz
Copy link
Author

MadJlzz commented Nov 2, 2018

@kazuki43zoo I've fixed all problems and I may have a solution. Instead of construction SqlSessionTemplate inside afterPropertiesSet, I initialize the object once if it is null.

That way, we could either use @Component and a basic constructor without having to call afterPropertiesSet.

Now, if you disagree with my point of view explained in #323, we can still abandon this PR and close the issue.

WDYT ?

@mp789991268
Copy link

Yes

@kazuki43zoo kazuki43zoo requested review from kazuki43zoo and removed request for kazuki43zoo April 3, 2019 14:59
@kazuki43zoo kazuki43zoo self-assigned this Apr 3, 2019
@kazuki43zoo kazuki43zoo added the enhancement Improve a feature or add a new feature label Apr 3, 2019
@kazuki43zoo kazuki43zoo added this to the 2.0.1 milestone Apr 3, 2019
@kazuki43zoo kazuki43zoo merged commit d807f27 into mybatis:master Apr 3, 2019
@kazuki43zoo
Copy link
Member

@MadJlzz I've merged now, Thanks for your contributing!

@kazuki43zoo kazuki43zoo removed their assignment Apr 3, 2019
@kazuki43zoo kazuki43zoo removed the enhancement Improve a feature or add a new feature label Apr 3, 2019
@kazuki43zoo kazuki43zoo removed this from the 2.0.1 milestone Apr 3, 2019
pulllock pushed a commit to pulllock/mybatis-spring that referenced this pull request Oct 19, 2023
Relax MyBatisPagingItemReader to allow constructor instanciation
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