lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael McCandless <luc...@mikemccandless.com>
Subject Re: Greetings and questions about patches
Date Thu, 23 Apr 2009 10:15:30 GMT
Welcome Erick!

Because nextHighestPowerOfTwo methods are public, I think we cannot
change what they return, nor remove them.  At most we could deprecate
them now (and remove in 3.0), though I think it's fine to simply keep
them around even though nothing inside Lucene uses them today: since
we are heavy users of BitSet/Vector/Array/etc., it seems possible
we'll need them at some point.

EG we are looking to make a better data structure to share
nearly-identical deleted doc bitsets between near realtime readers,
which could conceivably use advanced BitUtil methods.

Keep hacking ;)

Mike

On Wed, Apr 22, 2009 at 9:33 PM, Erick Erickson <erickerickson@gmail.com> wrote:
> Hi all:
>
> I've been participating in the user list for some time, and I'd like
> to start helping maintain/enhance the code. So I thought I'd start
> with something small, mostly to get the process down. Unit tests
> sure fit the bill it seems to me, less chance of introducing errors
> through ignorance but a fine way to extend *my* understanding
> of Lucene.
>
> I managed to check out the code and run the unit tests, which
> was amazingly easy. I even managed to get the project into
> IntelliJ and connect the codestyle.xml file. Kudos for whoever
> set up the checkout/build process, I was dreading spending
> days setting this up, fortunately I didn't have to.
>
> So I, with Chris's help, found the code coverage report and
> chose something pretty straightforward to test, BitUtil since it
> was nice and self-contained. As I said, I'm looking at understanding
> the process rather than adding much value the first time.
>
> Alas, even something as simple as BitUtil generates questions
> that I'm asking mostly to understand what approach the veterans
> prefer. I'll argue with y'all next year sometime <G>.
>
> So, according to the coverage report, there are two methods that
> are never executed by the unit tests (actually 4, 2 that operate on
> ints and 2 that operate on longs), isPowerOfTwo and
> nextHighestPowerOfTwo. nextHighestPowerOfTwo is especially
> clever, had to get out a paper and pencil to really understand it.
>
> Issues:
> 1> none of these methods is ever called. I commented them out
>      and ran all the unit tests and all is well. Additionally, commenting
>      out one of the other methods produces compile-time errors so I'm
>     fairly sure I didn't do something completely stupid that just *looked*
>     like it was OK. I grepped recursively and they're nowhere in the
>     *.java files.
>   1a> What's the consensus about unused code? Take it out (my
>          preference) along with leaving a comment on where it can
>          be found (since it *is* clever code)? Leave it in because someone
>          found some pretty neat algorithms that we may need sometime?
>   1b> I'm not entirely sure about the contrib area, but the contrib jars
>          are all new so I assume "ant clean test" compiles them as well.
>
> 2> I don't agree with the behavior of nextHighestPowerOfTwo. Should
>      I make changes if we decide to keep it?
>   2a> Why should it return the parameter passed in when it happens to be
>         a perfect power of two? e.g. this passes:
>        assertEquals(BitUtil.nextHighestPowerOfTwo(128L), 128);
>        I'd expect this to actually return 256, given the name.
> 2b> Why should it ever return 0? There's no power of two that is
>        zero. e.g. this passes:
>        assertEquals(BitUtil.nextHighestPowerOfTwo(-1), 0);
>        as does this: assertEquals(BitUtil.nextHighestPowerOfTwo(0), 0).
>        *Assuming* that someone wants to use this sometime to, say, size
>         an array they'd have to test against a return of 0.
>
>
> I'm fully aware that these are trivial issues in the grand scheme of things,
> and I *really* don't want to waste much time hashing them over. I'll provide
> a patch either way and go on to something slightly more complicated for
> my next trick.
>
> Best
> Erick
>

---------------------------------------------------------------------
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