Bug 96355

Summary: Create Localizer factory method for LocaleMac
Product: WebKit Reporter: Keishi Hattori <keishi>
Component: FormsAssignee: Keishi Hattori <keishi>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, tkent, webkit.review.bot, yosin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 96351    
Attachments:
Description Flags
Patch none

Keishi Hattori
Reported 2012-09-10 23:51:23 PDT
Create Localizer factory method for LocaleMac
Attachments
Patch (4.13 KB, patch)
2012-09-11 00:12 PDT, Keishi Hattori 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Keishi Hattori
Comment 1 2012-09-11 00:12:08 PDT
Created attachment 163288 [details] Patch
Kent Tamura
Comment 2 2012-09-11 00:17:28 PDT
Comment on attachment 163288 [details] Patch ok
WebKit Review Bot
Comment 3 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>
WebKit Review Bot
Comment 4 2012-09-11 01:15:00 PDT
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 5 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?
Keishi Hattori
Comment 6 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.
Alexey Proskuryakov
Comment 7 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.
Keishi Hattori
Comment 8 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?
Note You need to log in before you can comment on or make changes to this bug.