[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