[Webkit-unassigned] [Bug 34852] Refactor parsing/serialization functions in HTMLInputElement

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

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




--- Comment #2 from Darin Adler <darin at apple.com>  2010-02-12 09:51:50 PST ---
(From update of attachment 48582)
I would prefer that we use references rather than pointers for the out
arguments to all these functions.

> +        Refacotr parsing/serialization functions in HTMLInputElement.

Typo: Refacotr.

> +        value. parseToDoubleForNumer() and parseToDateComponents()

Typo: parseToDoubleForNuer.

> +        functions are helper functions for it. serializeForCurrentType()
> +        is the top-level function to serialize a double value to a
> +        type-dependent string, and serializeForNumberType() and
> +        serializeForDateTimeTypes() are helper functions for it.

I think serializeForCurrentType could be named just serialize.

> +    ASSERT_NOT_REACHED();
> +    return;
> +}

It's not good style to have return at the end of the function. I suggest just
leaving it out.

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

Including a default case defeats the GCC warning that lets us know if we forget
to add a new enumeration value to the switch statement. If possible you should
restructure so we don't have te default. In fact, I think the warning is
slightly more important than the assertion, so I'd be willing to sacrifice
that.

r=me

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