Summary: | webdatabase: Introducing WTF::TypeSafeEnum and DatabaseError | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||||
Component: | WebCore Misc. | Assignee: | Mark Lam <mark.lam> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, beidson, benjamin, cmarcelo, ggaren, haraken, japhet, ojan.autocc, sam, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 107475 | ||||||||
Attachments: |
|
Description
Mark Lam
2013-01-29 18:33:19 PST
Created attachment 185386 [details]
the patch.
Comment on attachment 185386 [details] the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=185386&action=review r=me > Source/WTF/wtf/TypeSafeEnum.h:51 > +// MyEnum value1; // value1 is assigned MyEnum::VALUE_DEFAULT by default. Typo: MyEnumDef. > Source/WebCore/Modules/webdatabase/DatabaseError.h:35 > +struct DatabaseErrorDef { We don't usually use abbreviations like "Def". How about calling this "DatabaseErrorStruct" or "DatabaseErrorDefinition"? Comment on attachment 185386 [details] the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=185386&action=review >> Source/WTF/wtf/TypeSafeEnum.h:51 >> +// MyEnum value1; // value1 is assigned MyEnum::VALUE_DEFAULT by default. > > Typo: MyEnumDef. This is not a typo. With the TypeSafeEnum template, you can say MyEnum::VALUE_DEFAULT and it gets redirected to MyEnumDef::VALUE_DEFAULT. ... which reminds me, I should change that to MyEnum::ValueDefault which is what the style checker says is preferred. I will update before committing. >> Source/WebCore/Modules/webdatabase/DatabaseError.h:35 >> +struct DatabaseErrorDef { > > We don't usually use abbreviations like "Def". How about calling this "DatabaseErrorStruct" or "DatabaseErrorDefinition"? Will change to DatabaseErrorDefinition. C++ 11 has strongly typed enum. Can't we use that and people without C++ 11 would have weak typed enums? That would make the code simpler. (In reply to comment #4) > C++ 11 has strongly typed enum. > > Can't we use that and people without C++ 11 would have weak typed enums? That would make the code simpler. The code is not all that complicated, and the type safety actually has an added benefit of scoping e.g. DatabaseError::None. Otherwise, I will have to add a prefix. I'm landing this for now, and will look into how we can abstract this to use C++11 enums if available later. > The code is not all that complicated, and the type safety actually has an added benefit of scoping e.g. DatabaseError::None. Otherwise, I will have to add a prefix. enum class are scoped. > I'm landing this for now, and will look into how we can abstract this to use C++11 enums if available later. I looked into supporting both but that makes things worse. :( Ignore me, go ahead with the patch. Landed in r141219: <http://trac.webkit.org/changeset/141219>. Reopening to attach new patch. Created attachment 185390 [details]
Patch
Comment on attachment 185390 [details]
Patch
sorry, wrong copy paste :(
|