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

Refactoring in findTargetWithGivenAcceptType #245

Closed
wants to merge 2 commits into from

Conversation

marcussoliva
Copy link

Hello,
I made a refactoring in findTargetWithGivenAcceptType method. All tests are passing, can validate please? Thank U.

Tests run: 69, Failures: 0, Errors: 0, Skipped: 0
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------

@perwendel
Copy link
Owner

I'm trying to validate. A question:
routeWithGivenAcceptType is not called anymore. Isn't that needed?

@marcussoliva
Copy link
Author

Hi,
I do not need routeWithGivenAcceptType.

MimeParse.bestMatch(acceptedMimeTypes.keySet(),acceptType); returns NO_MIME_TYPE if it fails and routeWithGivenAcceptType only verifies that the return of this operation is not equal to NO_MIME_TYPE to perform an operation on the map, I believe that this operation can be performed without any check as if the key does not exist on the map return the operation will be null.

Correct ?

@tipsy tipsy added the WL:Minor label Nov 23, 2015
if (acceptType != null && routeMatches.size() > 0) {

if (routeMatches.isEmpty()) {
return null;

Choose a reason for hiding this comment

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

can "routeMatches" can be null? if spark has apache commons use CollectionUtils.isEmpty() else, just check list is null too

@tipsy
Copy link
Contributor

tipsy commented Feb 19, 2017

Class has been deprecated.

@tipsy tipsy closed this Feb 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants