[Webkit-unassigned] [Bug 67081] Basic enhancements to StringBuilder

[bugzilla-daemon at webkit.org]

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





--- Comment #10 from Darin Adler <darin at apple.com>  2011-09-16 11:20:40 PST ---
(In reply to comment #7)
> (In reply to comment #6)
> > (From update of attachment 106970 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=106970&action=review
> > 
> > review- because of the mistake in the == functions
> > 
> > I have a question about the toString and toStringPreserveCapacity functions in this class. It seems that if you do this:
> > 
> >     builder.toString()
> >     builder.append(character)
> >     builder.toString()
> > 
> > and the buffer still has additional capacity remaining, then the second toString will return an incorrect result. I think this is a basic implementation flaw in the class as it currently stands.

> 2. m_string.impl() is a substring of m_buffer, and the append() doesn't exceed capacity. As append() will only change the content after original m_length, m_string will remain unchanged.

Right, that’s the problematic case. The second call to builder.toString() will return the same string as the first, a string that does not include the last appended character. Unless I’m missing something. Did you actually test this case?

> >> Source/JavaScriptCore/wtf/text/StringBuilder.h:40
> >> +    friend bool operator!=(const StringBuilder&, const StringBuilder&);
> > 
> > Only two of the functions need to be implemented as friend functions. The rest could just call the others. I’d prefer to do it that way to cut down on the number of friend functions.
> 
> I changed 4 of them (whose first parameter is StringBuilder) to StringBuilder methods, and the other two as inline functions that call the methods.

Normally it is best style to avoid having binary operators be member functions, and non-members functions are preferred, friend when necessary. Changing them to be member functions has a small downside, since type conversions can’t be done on the left side when the operator is a member function. Probably OK here, though.

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