Summary: | Add a nicer way to iterate over all the attributes of an element | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Benjamin Poulain <benjamin> | ||||
Component: | New Bugs | Assignee: | Benjamin Poulain <benjamin> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | ||||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Benjamin Poulain
2014-01-19 23:24:09 PST
Created attachment 221620 [details]
Patch
Comment on attachment 221620 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221620&action=review > Source/WebCore/dom/ElementData.h:73 > +class AttributeIteratorAccessor { > +public: > + AttributeIteratorAccessor(const Attribute* array, unsigned size) > + : m_array(array) > + , m_size(size) > + { > + } > + > + AttributeConstIterator begin() const { return AttributeConstIterator(m_array, 0); } > + AttributeConstIterator end() const { return AttributeConstIterator(m_array, m_size); } > +private: > + const Attribute* m_array; > + unsigned m_size; > +}; I think you can use WTF::IteratorRange here instead. Comment on attachment 221620 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221620&action=review r=me Please consider Anders's suggestion. > Source/WebCore/dom/ElementData.cpp:182 > + unsigned count = length(); In cases like this, I prefer "unsigned length = this->length()" or "unsigned length = ElementData::length()". It's bad when one thing becomes two. Taking "length" and renaming it to "count" makes it seem like you've changed it somehow, which can be confusing. (In reply to comment #2) > I think you can use WTF::IteratorRange here instead. My first idea was to just use the Attribute* as the iterator. But for some reason clang really sucks iterating with pointers :( Committed r162394: <http://trac.webkit.org/changeset/162394> |