[webkit-reviews] review denied: [Bug 3405] Replace Id for tag names with QualifiedName : [Attachment 2343] Add initial implementations of QualifiedName and the HTMLNames classes.

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Wed Jun 15 10:11:51 PDT 2005


Darin Adler <darin at apple.com> has denied Dave Hyatt <hyatt at apple.com>'s request
for review:
Bug 3405: Replace Id for tag names with QualifiedName
http://bugzilla.opendarwin.org/show_bug.cgi?id=3405

Attachment 2343: Add initial implementations of QualifiedName and the HTMLNames
classes.
http://bugzilla.opendarwin.org/attachment.cgi?id=2343&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Looks like this won't compile due to m_inner vs. m_impl and QualifiedNameImpl
vs. QualifiedNameInner.

The implementation file should be wrapped in namespace DOM rather than having a
using namespace DOM at the top of the file.

I suggest that m_qNameCache just be a private global inside the .cpp file
(using static) and not a member of the QualifiedName class; no need to expose
it, and then you don't have to include <qptrdict.h> in the header.

I prefer that you put spaces after the < when you have to put a space before
the > in expressions like:

QPtrDict< QPtrDict< QPtrDict<QualifiedNameInner> > >

Why all the (void*) casts? Those shouldn't be necessary. Are they for const
casting?

Can we change HTMLNames from a class to a namespace? The advantage of that is
that you can do a using on individual tags, and there's really nothing here
that calls for a class.

Other than these nitpicks, it looks great. I think I'll mark it review- just to
be pedantic.



More information about the webkit-reviews mailing list