lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Uwe Schindler (JIRA)" <>
Subject [jira] Commented: (LUCENE-2285) Code cleanup from all sorts of (trivial) warnings
Date Thu, 25 Feb 2010 18:32:27 GMT


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:
Index: src/test/org/apache/lucene/analysis/
--- src/test/org/apache/lucene/analysis/	(revision 916146)
+++ src/test/org/apache/lucene/analysis/	(working copy)
@@ -76,7 +76,7 @@
     int n=0;
     for (Object o : cs) {
-      assertTrue(cm.containsKey((char[]) o));
+      assertTrue(cm.containsKey(o));
     assertEquals(hm.size(), n);
Index: src/test/org/apache/lucene/analysis/
--- src/test/org/apache/lucene/analysis/	(revision 916146)
+++ src/test/org/apache/lucene/analysis/	(working copy)
@@ -475,7 +475,7 @@
     assertFalse(CharArraySet.EMPTY_SET.contains((Object) "foo"));
-    assertFalse(CharArraySet.EMPTY_SET.contains((Object) "foo".toCharArray()));
+    assertFalse(CharArraySet.EMPTY_SET.contains("foo".toCharArray()));

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:
>             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:
For additional commands, e-mail:

View raw message