lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Uwe Schindler (JIRA)" <j...@apache.org>
Subject [jira] Commented: (LUCENE-2285) Code cleanup from all sorts of (trivial) warnings
Date Sun, 28 Feb 2010 10:01:05 GMT

    [ https://issues.apache.org/jira/browse/LUCENE-2285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12839396#action_12839396
] 

Uwe Schindler commented on LUCENE-2285:
---------------------------------------

Hi Shai,

sorry, in my opinion the discussion seems to be stuck, as both of us have a personal opinion
about the casts and it seems that both of us always presenting the same arguments, so in my
opinion, someone else should jump into the discussion. My point is simple that I want to have
the casts for "documentation" purposes and "safety" reasons, so later changes in the code
will be obvious and anybody reading the code is able to follow. This only affects casts to
(long) and (int) <-> (char).

I presented my arguments: Especially for backwards compatibility, as long as the 2.9 branch
is maintained (the argument about casting int -> char in code that may get merged to 2.9
branch): I tested it and broke my 2.9 build: A JDK 5 compiler (even with -target 1.4 and -source
1.4) would use the Character methods taking int params, as the JDK5 has it in their rt.jar.
If you compile with a JDK 1.4 compiler, the compiler will automatically cast to char, as no
int method is available. I tested this in my 2.9 branch: i removed the casts at some places,
the code compiled fine (with 1.5 compiler), but when running the tests with JDK 1.4, MethodNotFound
exceptions occurred. When I also compiled with the 1.4 compiler, the resulting jar was fine.
So forcing the compiler to use (char) methods or (int) methods especially in those Unicode
stuff is important. Maybe you understand my argument now. And also when going from 1.5 to
1.6 (so compiling 1.5 code in 1.6) may break in the same way. So I prefer to tell the compiler
the method to use.

And for these reasons, I don't understand why you insist in removing those casts. Eclipse
declaring them as warnings is in my opinion wrong and should maybe an info message, as everybody
is free to add casts and not rely on automatic casting. The same affects autoboxing, if you
dont like autoboxing you should be free to explicitely say how the values should be converted.
As Lucenes Collectors are hotspots, automboxing without control may affect performance! So
adding explicit boxing and explicit casts is a good idea, although *YOU* think they are wrong
or unneeded.

As I dont use eclipse, I have no idea about the correct parameter; I would suggest to add
a @SuppressWarnings("foobar") for supressing those cast messages in the affected classes.
Would this be an option? You will get no warnings and I can preserve my casts.

If you have other improvements in non-generated code I would be happy to commit the patches.
I also merged your patch yesterday to the flex branch, so Mike and Robert can also use it
in the flexible indexing branch. So again thank you for the improvements.

> 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-remaining+generated.patch, LUCENE-2285-remaining.patch,
LUCENE-2285.patch, 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org


Mime
View raw message