Bug 96355 - Create Localizer factory method for LocaleMac
Summary: Create Localizer factory method for LocaleMac
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keishi Hattori
URL:
Keywords:
Depends on:
Blocks: 96351
  Show dependency treegraph
 
Reported: 2012-09-10 23:51 PDT by Keishi Hattori
Modified: 2012-09-11 21:34 PDT (History)
4 users (show)

See Also:


Attachments
Patch (4.13 KB, patch)
2012-09-11 00:12 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keishi Hattori 2012-09-10 23:51:23 PDT
Create Localizer factory method for LocaleMac
Comment 1 Keishi Hattori 2012-09-11 00:12:08 PDT
Created attachment 163288 [details]
Patch
Comment 2 Kent Tamura 2012-09-11 00:17:28 PDT
Comment on attachment 163288 [details]
Patch

ok
Comment 3 WebKit Review Bot 2012-09-11 01:14:57 PDT
Comment on attachment 163288 [details]
Patch

Clearing flags on attachment: 163288

Committed r128156: <http://trac.webkit.org/changeset/128156>
Comment 4 WebKit Review Bot 2012-09-11 01:15:00 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Alexey Proskuryakov 2012-09-11 11:12:32 PDT
There is something wrong with the design here. How can a "Locale" class inherit from a "Localizer" class? How can LocaleMac::create return a pointer of a different type?
Comment 6 Keishi Hattori 2012-09-11 19:07:33 PDT
(In reply to comment #5)
> There is something wrong with the design here. How can a "Locale" class inherit from a "Localizer" class? How can LocaleMac::create return a pointer of a different type?

I will rename LocaleMac, LocaleWin, and LocaleICU to LocalizerMac, LocalizerWin, LocalizerICU in the future.

LocaleMac, LocaleWin, and LocaleICU all inherited NumberLocalizer before I started working on this. And I renamed NumberLocalizer to Localizer. The grand plan is to collect all the localizer functions, not just number, into Localizer.

Localizer::create will be used instead of LocaleMac::create to get the right instance for each platform.
Comment 7 Alexey Proskuryakov 2012-09-11 21:13:11 PDT
I'm not sure if I'm in support of this direction.

Class names should be nouns, and disguising a verb for a noun is rarely a sign of a good design. A class is a collection of data and methods, and it should be easy to tell from class name what the data is. On the other hand, a class with a name like "manager" or "localizer" doesn't really tell what its object oriented essence is.
Comment 8 Keishi Hattori 2012-09-11 21:34:17 PDT
(In reply to comment #7)
> I'm not sure if I'm in support of this direction.
> 
> Class names should be nouns, and disguising a verb for a noun is rarely a sign of a good design. A class is a collection of data and methods, and it should be easy to tell from class name what the data is. On the other hand, a class with a name like "manager" or "localizer" doesn't really tell what its object oriented essence is.

How about LocalizedFormat, LocalizedFormatMac, LocalizedFormatWin, LocalizedFormatICU?