ticket:10944: Add ByteBuffer support for BreakIterator.

ticket:10944: Add ByteBuffer support for BreakIterator.

R=markus.icu@gmail.com

Committed: http://bugs.icu-project.org/trac/changeset/36036

[markus.icu, 2014-07-11 18:25:56]

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
File main/classes/core/src/com/ibm/icu/text/BreakDictionary.java (right):

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/BreakDictionary.java:56: } finally {
good!

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
File main/classes/core/src/com/ibm/icu/text/BreakIteratorFactory.java (right):

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/BreakIteratorFactory.java:3: * Copyright
(C) 2002-2012, International Business Machines Corporation and    *
I just noticed this, please look at all of the copyright headers in those files
that you modify.

The year range needs to be extended to include the current year. In this case,
to 2002-2014.

Also, personally, whenever I see and modify(!) this kind of copyright block, I
delete the trailing spaces+asterisk. They only invite stupid alignment mistakes.
You need not do that if you like the box.

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/BreakIteratorFactory.java:119: }
finally: ruleStream.close()

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
File main/classes/core/src/com/ibm/icu/text/RBBIDataWrapper.java (right):

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/RBBIDataWrapper.java:194: // Current
position in input stream.
in input stream -> in the buffer

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/RBBIDataWrapper.java:266: //     leave
the input stream, leave position unchanged.
input stream -> buffer

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/RBBIDataWrapper.java:270:
ICUBinary.skipBytes(bytes, This.fHeader.fTrie - pos);  // seek input stream from
end of
ditto

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/RBBIDataWrapper.java:282: bytes.reset(); 
                        // Move input stream back to marked position at
ditto

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/RBBIDataWrapper.java:284: //   "pos"
variable and the input stream are in
input stream -> buffer

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
File main/classes/core/src/com/ibm/icu/text/RuleBasedBreakIterator.java (right):

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/RuleBasedBreakIterator.java:87: public
static RuleBasedBreakIterator getInstanceFromCompiledRules(ByteBuffer bytes)
throws IOException {
We can't just add public API. It needs to be proposed and approved first.

If we need this because it's called by other library code, then replace the
@draft 54.1 with @internal for now. If we don't need it for library code, then
just add a TODO comment into the existing method and remove this overload.

Also, if and when we do add it as public API, it needs to say "@draft ICU 54.1".
See http://userguide.icu-project.org/design#TOC-ICU-API-compatibility

Also, in Java, in addition to @draft and @internal lines, we have standard
@provisional or @deprecated lines as well; and wherever we have @deprecated API
docs we also need an @Deprecated annotation. See other ICU4J methods for
examples. We should probably have those details in
http://userguide.icu-project.org/dev/codingguidelines but I don't see
"@provisional" there :-(

[roubert (google), 2014-07-14 15:04:08]

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
File main/classes/core/src/com/ibm/icu/text/BreakDictionary.java (right):

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/BreakDictionary.java:56: } finally {
On 2014/07/11 18:25:55, markus.icu wrote:
> good!

Done.

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
File main/classes/core/src/com/ibm/icu/text/BreakIteratorFactory.java (right):

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/BreakIteratorFactory.java:3: * Copyright
(C) 2002-2012, International Business Machines Corporation and    *
On 2014/07/11 18:25:55, markus.icu wrote:
> I just noticed this, please look at all of the copyright headers in those
files
> that you modify.
> 
> The year range needs to be extended to include the current year. In this case,
> to 2002-2014.
> 
> Also, personally, whenever I see and modify(!) this kind of copyright block, I
> delete the trailing spaces+asterisk. They only invite stupid alignment
mistakes.
> You need not do that if you like the box.

Done.

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/BreakIteratorFactory.java:119: }
On 2014/07/11 18:25:55, markus.icu wrote:
> finally: ruleStream.close()

Done by getByteBufferFromInputStream().

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
File main/classes/core/src/com/ibm/icu/text/RBBIDataWrapper.java (right):

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/RBBIDataWrapper.java:194: // Current
position in input stream.
On 2014/07/11 18:25:56, markus.icu wrote:
> in input stream -> in the buffer

Done.

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/RBBIDataWrapper.java:266: //     leave
the input stream, leave position unchanged.
On 2014/07/11 18:25:55, markus.icu wrote:
> input stream -> buffer

Done.

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/RBBIDataWrapper.java:270:
ICUBinary.skipBytes(bytes, This.fHeader.fTrie - pos);  // seek input stream from
end of
On 2014/07/11 18:25:55, markus.icu wrote:
> ditto

Done.

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/RBBIDataWrapper.java:282: bytes.reset(); 
                        // Move input stream back to marked position at
On 2014/07/11 18:25:56, markus.icu wrote:
> ditto

Done.

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/RBBIDataWrapper.java:284: //   "pos"
variable and the input stream are in
On 2014/07/11 18:25:55, markus.icu wrote:
> input stream -> buffer

Done.

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
File main/classes/core/src/com/ibm/icu/text/RuleBasedBreakIterator.java (right):

https://codereview.appspot.com/108450046/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/RuleBasedBreakIterator.java:87: public
static RuleBasedBreakIterator getInstanceFromCompiledRules(ByteBuffer bytes)
throws IOException {
On 2014/07/11 18:25:56, markus.icu wrote:
> If we need this because it's called by other library code, then replace the
> @draft 54.1 with @internal for now.

This is the case. Done.

[markus.icu, 2014-07-15 16:55:16]

https://codereview.appspot.com/108450046/diff/80001/main/classes/core/src/com...
File main/classes/core/src/com/ibm/icu/text/RuleBasedBreakIterator.java (right):

https://codereview.appspot.com/108450046/diff/80001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/RuleBasedBreakIterator.java:86: *
@internal
For @internal you also need to add an @deprecated API doc line, and an
@Deprecated annotation. We do this so that IDEs give callers yellow squiggles;
IDEs don't know about @internal.

See BreakIterator#getBreakInstance(ULocale, int), please copy verbatim.

[roubert (google), 2014-07-15 19:28:52]

https://codereview.appspot.com/108450046/diff/80001/main/classes/core/src/com...
File main/classes/core/src/com/ibm/icu/text/RuleBasedBreakIterator.java (right):

https://codereview.appspot.com/108450046/diff/80001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/text/RuleBasedBreakIterator.java:86: *
@internal
On 2014/07/15 16:55:15, markus.icu wrote:
> For @internal you also need to add an @deprecated API doc line, and an
> @Deprecated annotation. We do this so that IDEs give callers yellow squiggles;
> IDEs don't know about @internal.
> 
> See BreakIterator#getBreakInstance(ULocale, int), please copy verbatim.

Done.

[markus.icu, 2014-07-15 19:40:47]

LGTM

[roubert (google), 2014-07-15 20:40:54]

Committed patchset #6 manually as r36036 (presubmit successful).