[Webkit-unassigned] [Bug 154003] Implement ComposedTreeIterator in terms of ElementAndTextDescendantIterator

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Feb 8 15:06:42 PST 2016


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

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #270878|review?                     |review+
              Flags|                            |

--- Comment #5 from Darin Adler <darin at apple.com> ---
Comment on attachment 270878
  --> https://bugs.webkit.org/attachment.cgi?id=270878
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=270878&action=review

r=me assuming you find and fix the assertion failure (or maybe just a crash) that the mac-debug bot is showing.

> Source/WebCore/dom/ComposedTreeIterator.h:161
> +        Iterator()
> +            : ComposedTreeIterator()
> +        { }

I think you can write:

    Iterator() = default;

Which I think is better.

> Source/WebCore/dom/ComposedTreeIterator.h:162
>          Iterator(ContainerNode& root)

explicit?

> Source/WebCore/dom/ComposedTreeIterator.h:177
> +    Iterator begin() { return Iterator(m_parent); }

Since the constructor is not explicit, could just write:

    return m_parent;

> Source/WebCore/dom/ComposedTreeIterator.h:178
> +    Iterator end() { return Iterator(); }

I slightly prefer:

    return { };

> Source/WebCore/dom/ElementAndTextDescendantIterator.h:36
> +class ElementAndTextDescendantIterator {

This class has a lot of code. Any way for it to share more with other classes?

> Source/WebCore/dom/ElementAndTextDescendantIterator.h:38
> +    ElementAndTextDescendantIterator();

I prefer = default.

> Source/WebCore/dom/ElementAndTextDescendantIterator.h:39
> +    ElementAndTextDescendantIterator(ContainerNode& root);

explicit?

> Source/WebCore/dom/ElementAndTextDescendantIterator.h:52
> +    bool operator!() const { return !m_current; }

I normally expect an explicit operator bool any place I see an operator!

> Source/WebCore/dom/ElementAndTextDescendantIterator.h:86
> +    ElementAndTextDescendantIteratorAdapter(ContainerNode& root);

explicit?

> Source/WebCore/dom/ElementAndTextDescendantIterator.h:99
> +    : m_current(nullptr)

Why not initialize this in the class definition?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160208/7a12b075/attachment-0001.html>


More information about the webkit-unassigned mailing list