Refactor WordIterator into base::BreakIterator....

Refactor WordIterator into base::BreakIterator.

BUG=49747
TEST=BreakIteratorTest.*

[Tom Sepez, 2010-12-13 20:05:46]

Latest round of my word_iterator project.

[Evan Martin, 2010-12-13 20:26:04]

LGTM

http://codereview.chromium.org/5796003/diff/1/base/i18n/break_iterator.h
File base/i18n/break_iterator.h (right):

http://codereview.chromium.org/5796003/diff/1/base/i18n/break_iterator.h#newc...
base/i18n/break_iterator.h:39: namespace base {  // Prevent conflict with ICU
BreakIterator
I guess this comment isn't necessary -- it's implicit that stuff in base belongs
in base.

http://codereview.chromium.org/5796003/diff/1/base/i18n/break_iterator.h#newc...
base/i18n/break_iterator.h:71: bool IsWord() const;
Maybe the comments on this function need to be updated, now that you have this
new SPACE version?

[Tom Sepez, 2010-12-13 21:13:58]

Interesting question.  IsWord() has meaning only for the word iterator, since
the underlying function that it calls (getRuleStatus) returns different enum
constants based on the type of the iterator.  So calling IsWord() on anything
other than a BREAK_WORD iterator results in a situation where there's an
apples-to-oranges comparison, e.g. UBRK_WORD_NONE is defined as 0 and
UBRK_LINE_SOFT happens to be defined as 0, so the function just happens to
return whether you've hit an EOL.

My feelings are to comment this along the lines of "applicable only to
BREAK_WORD iterators, false otherwise." and clamp the return value to false when
its the wrong type.   At the present time, this isn't used against BREAK_SPACE
in the code (outside of test suite).

[Evan Martin, 2010-12-13 21:27:07]

On 2010/12/13 21:13:58, Tom Sepez wrote:
> Interesting question.  IsWord() has meaning only for the word iterator, since
> the underlying function that it calls (getRuleStatus) returns different enum
> constants based on the type of the iterator.  So calling IsWord() on anything
> other than a BREAK_WORD iterator results in a situation where there's an
> apples-to-oranges comparison, e.g. UBRK_WORD_NONE is defined as 0 and
> UBRK_LINE_SOFT happens to be defined as 0, so the function just happens to
> return whether you've hit an EOL.
> 
> My feelings are to comment this along the lines of "applicable only to
> BREAK_WORD iterators, false otherwise." and clamp the return value to false
when
> its the wrong type.   At the present time, this isn't used against BREAK_SPACE
> in the code (outside of test suite).

Sounds good.

[Tom Sepez, 2010-12-13 22:05:06]

Also renamed the increasingly inaccurately named GetWord() method.

[Evan Martin, 2010-12-13 22:17:49]

LGTM