code review requested -- Arrays.binarySearch

0 views
Skip to first unread message

John Tamplin

unread,
Apr 12, 2007, 6:06:27 PM4/12/07
to Kelly Norton, Google Web Toolkit Contributors
This fixes issue 887, and is essentially sandymac's patch with comments and test cases.

This change is in /changes/jat/jre-binarysearch and was forked from r841.  You can switch a clean working copy to it with:
or you can merge the changes into a recent trunk with:

Note that I had to merge in the changes made in r844 so it would build.  If you want to see only the changes made here, you can merge out these changes with:
from a switch copy (it shouldn't be necessary if you merged the changes into trunk as they match).

Suggested commit log if you deem this acceptable:

Implement Arrays.binarySearch for primitive and object arrays (including a Comparator-based version for object arrays).  Based on Josh Bloch's sample code at
http://googleresearch.blogspot.com/2006/06/extra-extra-read-all-about-it-nearly.html.

Fixes Issue: 887
Patch by: sandymac, jat
Review by: knorton

--
John A. Tamplin
Software Engineer, Google

John Tamplin

unread,
Apr 12, 2007, 6:14:51 PM4/12/07
to Kelly Norton, Google Web Toolkit Contributors
On 4/12/07, John Tamplin <j...@google.com> wrote:
This fixes issue 887, and is essentially sandymac's patch with comments and test cases.

This change is in /changes/jat/jre-binarysearch and was forked from r841.  You can switch a clean working copy to it with:
or you can merge the changes into a recent trunk with:

Should have been:

Sandy McArthur

unread,
Apr 12, 2007, 9:41:32 PM4/12/07
to Google-Web-Tool...@googlegroups.com
For the binarySearch(Object[], Object) version, the "official" term
for "default comparison function" is "natural ordering".

Otherwise, looks great to me.


--
Sandy McArthur

"He who dares not offend cannot be honest."
- Thomas Paine

John Tamplin

unread,
Apr 13, 2007, 1:27:13 PM4/13/07
to Google-Web-Tool...@googlegroups.com
On 4/12/07, Sandy McArthur <sand...@gmail.com> wrote:
For the binarySearch(Object[], Object) version, the "official" term
for "default comparison function" is "natural ordering".

Branch updated in r864 to correct this wording and the copyright header.

Kelly Norton

unread,
Apr 13, 2007, 2:31:09 PM4/13/07
to Google-Web-Tool...@googlegroups.com
LGTM. I'm just going to make some changes to the javadoc so it works well with our doctool and then commit it.

/kel

Kelly Norton

unread,
Apr 13, 2007, 3:39:01 PM4/13/07
to Google-Web-Tool...@googlegroups.com
Committed as r865. I did some small touchups.

/kel

John Tamplin

unread,
Apr 13, 2007, 3:46:16 PM4/13/07
to Google-Web-Tool...@googlegroups.com
On 4/13/07, Kelly Norton <kno...@google.com> wrote:
Committed as r865. I did some small touchups.

Thanks.

Sandy McArthur

unread,
May 7, 2007, 6:01:40 PM5/7/07
to Google-Web-Tool...@googlegroups.com
On 4/13/07, John Tamplin <j...@google.com> wrote:
> On 4/13/07, Kelly Norton <kno...@google.com> wrote:
> > Committed as r865. I did some small touchups.

I found two emulation bugs in this code that was committed described
in comment 7 and 8 of issue 887:
http://code.google.com/p/google-web-toolkit/issues/detail?id=887#c7

I've included a fix for these and added Collections.binarySearch
methods in a patch attached to issue 1018:
http://code.google.com/p/google-web-toolkit/issues/detail?id=1018

Reply all
Reply to author
Forward
0 new messages