[Webkit-unassigned] [Bug 29004] [HTML5][Forms] simple implementation of date&time types of INPUT element

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=29004


Eric Seidel <eric at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #39342|review?                     |review-
               Flag|                            |




--- Comment #10 from Eric Seidel <eric at webkit.org>  2009-09-21 10:50:30 PDT ---
(From update of attachment 39342)
Explicit String construction isn't needed:
String("button")
the compiler will find String(char*) anyway.

Why two lookups?
 333     if (!t.isNull() && typeMap->contains(t))  // contains() with a null
string makes crash.
 334         newType = typeMap->get(t);
just do one get instead of two lookups.

Lets make a hash for these too:
@@ const AtomicString& HTMLInputElement::formControlType() const

Then you don't have a zillion DEFINE_STATIC_LOCAL calls, because the Hash can
be TYPE -> AtomicString

(That doesn't have to be part of this patch, but if you're in there cleaning
stuff up, you should consider it.  I think it will be much less code.  Heck, it
could even be a Vector instead of a Hash if you like.)

Default is bad with switches over enums:
     default:
 1048         return false;
it's breaks the compiler's ability to tell you when you're missing cases.  In
this function  1028 bool HTMLInputElement::isTextField() const it seems we
don't even want a switch.  We just want a single if, or?  Or at least we want
an explicit list of the false cases if you're going to use a switch.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.