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

Introduce a Connection#createChannel alternative that returns an optional #431

Closed
michaelklishin opened this issue Dec 13, 2018 · 5 comments
Assignees
Milestone

Comments

@michaelklishin
Copy link
Member

This is a follow-up to #430. What if Connection#createChannel returned an optional, so instead of a null when the max channel is reached it would be an empty result.

@acogoluegnes feel free to close this if you don't think breaking the API is worth the trouble here.

@michaelklishin michaelklishin added this to the 6.0.0 milestone Dec 13, 2018
@acogoluegnes
Copy link
Contributor

Returning Optional is a good fit if we want to strongly tell the user that Connection#createChannel can return null, this will make them deal with the absence of the channel every time. In theory and in the case of a brand new API, it would be a no-brainer I guess.

But here we have to consider it's a major API change, as it will affect applications as well as libraries depending on the Java client (Spring AMQP, Reactor RabbitMQ, PerfTest, etc)... including all our tests :-) And in practice, does Connection#createChannel return null very often? I don't think so, but with Optional, this would have to be taken into account every time (note Optional provides several ways to deal with the absence of value and they are more than decent).

It's definitely worth considering but it's not an easy question, let's sleep a bit on it. Note trying the change and updating all the tests would be a good to experiment.

@michaelklishin
Copy link
Member Author

Of course it's a very rare occurrence for Connection#createChannel to return a null, which is my most important concern: converting to an optional will affect every single caller. Perhaps we could introduce a new method as an alternative?

@michaelklishin michaelklishin removed this from the 6.0.0 milestone Dec 13, 2018
@acogoluegnes
Copy link
Contributor

This is another option, but we'll have to come up with an appropriate name for it.

@michaelklishin
Copy link
Member Author

michaelklishin commented Dec 13, 2018

How about openChannel which would have a JavaDoc link to the createChannel we already have, and vice versa?

@michaelklishin michaelklishin changed the title Convert Connection#createChannel to return an optional Introduce a Connection#createChannel alternative that returns an optional Dec 13, 2018
@acogoluegnes
Copy link
Contributor

That sounds good to me.

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

No branches or pull requests

2 participants