ticket:10944: Add ByteBuffer support for UCharacterNameReader.

ticket:10944: Add ByteBuffer support for UCharacterNameReader.

R=markus.icu@gmail.com

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

[markus.icu, 2014-07-11 20:11:35]

https://codereview.appspot.com/112960043/diff/1/main/classes/core/src/com/ibm...
File main/classes/core/src/com/ibm/icu/impl/UCharacterName.java (right):

https://codereview.appspot.com/112960043/diff/1/main/classes/core/src/com/ibm...
main/classes/core/src/com/ibm/icu/impl/UCharacterName.java:1175: is.close();
redundant, already closed (sorry for earlier comments for the opposite)

https://codereview.appspot.com/112960043/diff/1/main/classes/core/src/com/ibm...
File main/classes/core/src/com/ibm/icu/impl/UCharacterNameReader.java (right):

https://codereview.appspot.com/112960043/diff/1/main/classes/core/src/com/ibm...
main/classes/core/src/com/ibm/icu/impl/UCharacterNameReader.java:12: import
com.ibm.icu.util.VersionInfo;
should not be necessary

https://codereview.appspot.com/112960043/diff/1/main/classes/core/src/com/ibm...
main/classes/core/src/com/ibm/icu/impl/UCharacterNameReader.java:117:
VersionInfo id = VersionInfo.getInstance(
Please don't construct temporary VersionInfo objects. For the data format id
please either create an int or compare the bytes to the pieces of
DATA_FORMAT_ID_. For the data version, just check  dataformatversion[0] == 1.
See isAcceptable() in
http://bugs.icu-project.org/trac/browser/icu/trunk/source/common/unames.cpp

https://codereview.appspot.com/112960043/diff/1/main/classes/core/src/com/ibm...
main/classes/core/src/com/ibm/icu/impl/UCharacterNameReader.java:162: private
static final VersionInfo DATA_FORMAT_VERSION_ =
I suggest you just delete this, and test for the first byte==1, as I said above.

[roubert (google), 2014-07-14 15:05:45]

https://codereview.appspot.com/112960043/diff/1/main/classes/core/src/com/ibm...
File main/classes/core/src/com/ibm/icu/impl/UCharacterName.java (right):

https://codereview.appspot.com/112960043/diff/1/main/classes/core/src/com/ibm...
main/classes/core/src/com/ibm/icu/impl/UCharacterName.java:1175: is.close();
On 2014/07/11 20:11:35, markus.icu wrote:
> redundant, already closed (sorry for earlier comments for the opposite)

Done.

https://codereview.appspot.com/112960043/diff/1/main/classes/core/src/com/ibm...
File main/classes/core/src/com/ibm/icu/impl/UCharacterNameReader.java (right):

https://codereview.appspot.com/112960043/diff/1/main/classes/core/src/com/ibm...
main/classes/core/src/com/ibm/icu/impl/UCharacterNameReader.java:12: import
com.ibm.icu.util.VersionInfo;
On 2014/07/11 20:11:35, markus.icu wrote:
> should not be necessary

Done.

https://codereview.appspot.com/112960043/diff/1/main/classes/core/src/com/ibm...
main/classes/core/src/com/ibm/icu/impl/UCharacterNameReader.java:117:
VersionInfo id = VersionInfo.getInstance(
On 2014/07/11 20:11:35, markus.icu wrote:
> Please don't construct temporary VersionInfo objects. For the data format id
> please either create an int or compare the bytes to the pieces of
> DATA_FORMAT_ID_. For the data version, just check  dataformatversion[0] == 1.
> See isAcceptable() in
> http://bugs.icu-project.org/trac/browser/icu/trunk/source/common/unames.cpp

Done.

https://codereview.appspot.com/112960043/diff/1/main/classes/core/src/com/ibm...
main/classes/core/src/com/ibm/icu/impl/UCharacterNameReader.java:162: private
static final VersionInfo DATA_FORMAT_VERSION_ =
On 2014/07/11 20:11:35, markus.icu wrote:
> I suggest you just delete this, and test for the first byte==1, as I said
above.

Done.

[markus.icu, 2014-07-15 18:38:21]

LGTM

https://codereview.appspot.com/112960043/diff/40001/main/classes/core/src/com...
File main/classes/core/src/com/ibm/icu/impl/UCharacterNameReader.java (right):

https://codereview.appspot.com/112960043/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/impl/UCharacterNameReader.java:152: * No
guarantees are made if a older version is used
Please delete this line.

[roubert (google), 2014-07-15 19:29:21]

https://codereview.appspot.com/112960043/diff/40001/main/classes/core/src/com...
File main/classes/core/src/com/ibm/icu/impl/UCharacterNameReader.java (right):

https://codereview.appspot.com/112960043/diff/40001/main/classes/core/src/com...
main/classes/core/src/com/ibm/icu/impl/UCharacterNameReader.java:152: * No
guarantees are made if a older version is used
On 2014/07/15 18:38:21, markus.icu wrote:
> Please delete this line.

Done.

[markus.icu, 2014-07-15 19:50:59]

LGTM++

[roubert (google), 2014-07-15 21:08:36]

Committed patchset #4 manually as r36040 (presubmit successful).