Skip to content

Cleanup IDE warnings #1227

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 1 commit into from
Aug 11, 2016
Merged

Cleanup IDE warnings #1227

merged 1 commit into from
Aug 11, 2016

Conversation

doom369
Copy link
Contributor

@doom369 doom369 commented Aug 11, 2016

Motivation :

IDE highlights a lot of warnings, code hard to read with them.

Modifications :

Removed unnecessary explicit type arguments, final for private methods, replaces for with for each.

Result :

Cleaner code.

@@ -63,8 +63,7 @@ public static String encode(Collection<Cookie> cookies) {
} else {
Cookie[] cookiesSorted = cookies.toArray(new Cookie[cookies.size()]);
Arrays.sort(cookiesSorted, COOKIE_COMPARATOR);
for (int i = 0; i < cookiesSorted.length; i++) {
Cookie cookie = cookiesSorted[i];
for (Cookie cookie : cookiesSorted) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I would tend to disagree here: the original form saves an Iterator. One could argue JIT escape analysis might be able to save it, but maybe not. Netty does the same thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@slandelle what iterator do you mean? This is array, for each in arrays don't use iterator. Compiler extract this loop into your version, It has nothing to do with JIT. I can show you bytecode if you want :).

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, right!

@slandelle
Copy link
Contributor

LGTM except for one small nit. Could you please revert this one single change?

@slandelle slandelle merged commit 34c46bd into AsyncHttpClient:master Aug 11, 2016
@slandelle
Copy link
Contributor

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants