[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





--- Comment #5 from TAMURA, Kent <tkent at chromium.org>  2009-11-18 17:37:07 PST ---
I'll commit manually with the following changes.

(In reply to comment #4)
> (From update of attachment 43345 [details])
> > +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".

Renamed to "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.

Renamed to "ignoredResult."

> > +    const UChar* uchars = src.characters();
> 
> Personally, I'd just name this "characters".

Done.

> > +    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.

Moved "return" to the default section.


Thanks!

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