[
https://issues.apache.org/jira/browse/LUCENE-2285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12838708#action_12838708
]
Shai Erera commented on LUCENE-2285:
------------------------------------
Uwe, your examples are still wrong. CharArrayMap has 3 methods:
containsKey(CharSequence), containsKey(Object) and containsKey(char[], int,
int). There is no contains(char[]). Therefore when you cast to char[], the
Object method is the one that's called, not that char[],int,int.
If I change the code to: assertTrue(cm.containsKey((char[]) o, 0, ((char[])
o).length )); then the right method is invoked. So I guess the tests were
defected in the first place .. and like I said, eclipse doesn't lie when it
says a cast is unnecessary, at least I haven't seen such a case yet.
I'll fix those two tests now, because they were defective right from the
beginning. Thanks for spotting this, because you've just revealed a bug which
existed in the tests :).
bq. because of the same we don't want to have autoboxing in internal lucene code
I don't see how autoboxing is related to casting ... If a map returns an
Integer, and you assign it to 'int', then whether or not you'll do the cast it
will autounbox it. If you assign it to an Integer, then you won't be able to
cast to 'int' (I think?) and hence the cast is redundant as well.
About Character methods, eclipse is smart enough to detect that when you call a
method w/ a char type, then the right one is called, vs. if you call it with
the int type. Hovering over the method call reveals immediately the method
variant that's called. So I see no reason why a char would be need to cast to
(char). If you want to call an int variant method, then you'll need to cast to
int, and eclipse won't complain about that.
Switching off compiler warnings in eclipse is your choice ... the Lucene code
is full of 'hidden' casting because that's how Java works. When you do 'int' *
1.0, it's cast to double, and people are aware of that ... in fact, they have
to assign the result to a double, or they'll be forced to cast to anything
else. When you work w/ integers and the method returns a long, it's cast
automatically. So if there are few cases where you'd want to alert the user,
put it in a comment, or like int charint = /*(int)*/ c;
Like I said, it's a styling issue. I'm not going to turn off my warnings in
eclipse ... and I don't understand what's this 'generified instanceof checks" -
can you give an example?
> 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, 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]
