[webkit-reviews] review denied: [Bug 92976] Move number localization code in LocaleICU.cpp to new class : [Attachment 156041] Patch

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has denied Kent Tamura
<tkent at chromium.org>'s request for review:
Bug 92976: Move number localization code in LocaleICU.cpp to new class
https://bugs.webkit.org/show_bug.cgi?id=92976

Attachment 156041: Patch
https://bugs.webkit.org/attachment.cgi?id=156041&action=review

------- Additional Comments from Kentaro Hara <haraken at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=156041&action=review


r- due to a wrong copyright.

> Source/WebCore/platform/text/NumberLocalizer.cpp:2
> + * Copyright (C) 2011 Google Inc. All rights reserved.

2012

> Source/WebCore/platform/text/NumberLocalizer.h:13
> + * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
AND

Please use Google's copyright.

> Source/WebCore/platform/text/NumberLocalizer.h:54
> +    String m_positivePrefix;
> +    String m_positiveSuffix;
> +    String m_negativePrefix;
> +    String m_negativeSuffix;
> +    bool m_hasNumberLocalizerData;

It looks a bit buggy to set these values one by one in the initialization
method of a sub class (i.e. LocaleICU::initializeNumberLocalizerData()).

How about implementing NumberLocalizer::initializeNumberLocalizerData(String
positivePrefix, String positiveSuffix, ...) that sets all data and flips
m_hasNumberLocalizerData? LocaleICU::initializeNumberLocalizerData() can call
NumberLocalizer::initializeNumberLocalizerData().