[Webkit-unassigned] [Bug 31330] Expose dtoa() for ECMA-262

[bugzilla-daemon at webkit.org]

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





--- Comment #10 from TAMURA, Kent <tkent at chromium.org>  2009-11-18 19:06:23 PST ---
I'll land this manually with the following changes.

(In reply to comment #9)
> (From update of attachment 43340 [details])
> > +    UChar* d;
> > +    if (!allocChars(length).getValue(d))
> > +        return &UString::Rep::null();
> > +    else {
> > +        for (unsigned i = 0; i < length; i++)
> > +            d[i] = static_cast<unsigned char>(c[i]); // use unsigned char to zero-extend instead of sign-extend
> > +        return UString::Rep::create(d, static_cast<int>(length));
> > +    }
> > +
> > +}
> 
> Normally we do early return, not else after return. There's also an extra blank
> line here.

Fixed them.

> Is the static_cast<int> really needed?

Changed the unsigned parameter to int.  So it is not needed.

> I suggest marking this createRep function, which is used in only one place,
> inline.

Done.

> > +        UString(const char*, unsigned length);
> >          UString(const UChar*, int length);
> >          UString(UChar*, int length, bool copy);
> 
> Given that the other constructors take int for length, why did you chose
> unsigned? If this was a new class, I would probably use size_t or perhaps
> unsigned, but typically we just keep new consistent with old, locally.

Change it to int.

> > +    void doubleToStringInJavaScriptFormat(double, DtoaBuffer buffer, unsigned* resultLength);
> >  } // namespace WTF
> 
> Probably would be slightly nicer to have a blank line before the closing brace.

Done.

> Could omit the name "buffer" since the type name already says that.

Done.

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