[
https://issues.apache.org/jira/browse/LUCENE-2285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12838464#action_12838464
]
Uwe Schindler commented on LUCENE-2285:
---------------------------------------
bq. The removed cast from TestCharArraySet is justified because you want to
test the contains(Object) method, which is exactly what happens. In fact, when
I look at the code, I think there is a wrong cast:
This is not true:
{noformat}
Index: src/test/org/apache/lucene/analysis/TestCharArrayMap.java
===================================================================
--- src/test/org/apache/lucene/analysis/TestCharArrayMap.java (revision
916146)
+++ src/test/org/apache/lucene/analysis/TestCharArrayMap.java (working copy)
@@ -76,7 +76,7 @@
int n=0;
for (Object o : cs) {
assertTrue(cm.containsKey(o));
- assertTrue(cm.containsKey((char[]) o));
+ assertTrue(cm.containsKey(o));
n++;
}
assertEquals(hm.size(), n);
Index: src/test/org/apache/lucene/analysis/TestCharArraySet.java
===================================================================
--- src/test/org/apache/lucene/analysis/TestCharArraySet.java (revision
916146)
+++ src/test/org/apache/lucene/analysis/TestCharArraySet.java (working copy)
@@ -475,7 +475,7 @@
assertFalse(CharArraySet.EMPTY_SET.contains(stopword));
}
assertFalse(CharArraySet.EMPTY_SET.contains((Object) "foo"));
- assertFalse(CharArraySet.EMPTY_SET.contains((Object) "foo".toCharArray()));
+ assertFalse(CharArraySet.EMPTY_SET.contains("foo".toCharArray()));
assertFalse(CharArraySet.EMPTY_SET.contains("foo".toCharArray(),0,3));
}
{noformat}
The problem here is:
We have a char[] and a Object method. The check tests that also the Object
method accepts char[]. This is important if you cast CharArraySet to
java.util.Set and call with char[]. So removin the cast for this test is simply
wrong. You can check this with Clover. And you patch even adds the same check
two times - instead of forcing the right method.
And by the way: When I run "ant" and it compiles I get no warning message at
all.
bq. Are you sure? I have another project which compiles w/ javac and it works
fine. I'll check it, but I trust you .
As I said before, java compiles need to simply ignore unknown SuppressWarnings
(see Java Language specs). It's only Eclipse that is to over
bq. About adding casts for clarity of code - I guess that's a matter of
styling, but the cast is truly unnecessary, and just produces a warning. I
would like the code to be free of those, but that's only my opinion.
Yes its a matter of styling, because of the same we don't want to have
autoboxing in internal lucene code, because autoboxing has a speed impact for
some of Lucene's code (like collectors). So we want to see what happens.
I want to understand that I apply a char -> int conversion, especially in our
new TokenFilters you can get a problem very fast as Character-methods are very
sensitive if called with char or int from the Unicode part. And I say it again,
javac shows no warning, so I don't see any need to change this, just because
this Eclipse prints a useless warning. But you can switch them off. I have some
warnings i simply swicth off after creating a project with eclipse. Like the
problem with generified instanceof checks, which are a bug in Eclipse.
> Code cleanup from all sorts of (trivial) warnings
> -------------------------------------------------
>
> Key: LUCENE-2285
> URL: https://issues.apache.org/jira/browse/LUCENE-2285
> Project: Lucene - Java
> Issue Type: Improvement
> Reporter: Shai Erera
> Assignee: Uwe Schindler
> Priority: Minor
> Fix For: 3.1
>
> Attachments: LUCENE-2285.patch
>
>
> I would like to do some code cleanup and remove all sorts of trivial
> warnings, like unnecessary casts, problems w/ javadocs, unused variables,
> redundant null checks, unnecessary semicolon etc. These are all very trivial
> and should not pose any problem.
> I'll create another issue for getting rid of deprecated code usage, like
> LuceneTestCase and all sorts of deprecated constructors. That's also trivial
> because it only affects Lucene code, but it's a different type of change.
> Another issue I'd like to create is about introducing more generics in the
> code, where it's missing today - not changing existing API. There are many
> places in the code like that.
> So, with you permission, I'll start with the trivial ones first, and then
> move on to the others.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
