Skip to content

Added getFirstMappedPort method#377

Merged
rnorth merged 3 commits intomasterfrom
no_args_getMappedPort
Jun 27, 2017
Merged

Added getFirstMappedPort method#377
rnorth merged 3 commits intomasterfrom
no_args_getMappedPort

Conversation

@bsideup
Copy link
Copy Markdown
Member

@bsideup bsideup commented Jun 23, 2017

This change simplifies the usage for 90% cases where users have something like:

@Rule
GenericContainer container = new GenericContainer().withExposedPorts(1234)

// ...

container.getMappedPort(1234)

by introducing new method:

container.getFirstMappedPort()

and it is just a safer version of:

container.getMappedPort(container.getExposedPorts().get(0))

It helps to remove some boilerplate, as well as "magic number" duplication.

@bsideup bsideup added this to the 1.4.0 milestone Jun 23, 2017
@bsideup bsideup requested a review from rnorth June 23, 2017 12:53
@bsideup bsideup changed the title Added no-args getMappedPort overloaded method Added getFirstMappedPort method Jun 23, 2017
* @return the port that the exposed port is mapped to
* @throws IllegalStateException if there are no exposed ports
*/
default Integer getFirstMappedPort() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 is use of streams absolutely essential here? 😉

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, at least I wouldn't say it's a misuse :)

Also, I prefer findFirst + map over !isEmpty() + .get(0), less error prone if you ask me

@rnorth
Copy link
Copy Markdown
Member

rnorth commented Jun 25, 2017

Seems a very good idea- as you say for most usages this is going to simplify usage.

@rnorth rnorth merged commit ed63472 into master Jun 27, 2017
@rnorth rnorth deleted the no_args_getMappedPort branch June 27, 2017 19:51
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.

2 participants