[Webkit-unassigned] [Bug 31342] Type constraint check for date&time types

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #43345|review?                     |review+
               Flag|                            |




--- Comment #4 from Darin Adler <darin at apple.com>  2009-11-18 15:26:51 PST ---
(From update of attachment 43345)
> +bool HTMLInputElement::formStringToISODateTime(InputType type, const String& src, ISODateTime* out)

I'd name it "formString" or "string" rather than "src", since the name of the
function says "formString".

> +    ISODateTime result;
> +    if (!out)
> +        out = &result;

I don't think "result" is a great name for this local buffer used when the
result is not wanted. Maybe "ignoredResult" would be better. Or even
"localBuffer" or something like that.

> +    const UChar* uchars = src.characters();

Personally, I'd just name this "characters".

> +    default:
> +        ASSERT_NOT_REACHED();
> +    }
> +    return false;

I'd put the return in with the default, or put the ASSERT out with the return.
Normally I also put in all the other cases, because I like the way gcc will
warn you if you add a new enum and don't have a case for it, but that would be
a long list of cases.

r=me as is

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