[Webkit-unassigned] [Bug 218298] Make WebCore::ContainerNode::ChildChangeType enum class

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Oct 28 12:51:18 PDT 2020


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

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |darin at apple.com
 Attachment #412550|review?                     |review+
              Flags|                            |

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

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

I’m going to say review+ even though I am not *sure* this is an improvement.

> Source/WebCore/dom/ContainerNode.h:76
> +    enum class ChildChangeType: uint8_t { ElementInserted, ElementRemoved, TextInserted, TextRemoved, TextChanged, AllChildrenRemoved, NonContentsChildRemoved, NonContentsChildInserted, AllChildrenReplaced };

Not sure this is an improvement. I understand that we like the idea of using enum class everywhere, but this makes all the code wordier and it’s not *obvious* to me that it’s better.

We know it’s better when the actual enumeration values can have shorter more straightforward names because the enumeration’s name will always be cited just before them. But here we were not able to do that.

Maybe this enumeration should be a in the ChildChange structure, with the name "Type" instead of next to the ChildChange structure with the name ChildChangeType.

Note that we can set the base type to uint8_t even without switching from enum to enum class.

> Source/WebCore/dom/ContainerNode.h:77
>      enum class ChildChangeSource { Parser, API };

Should probably put a base type of bool here?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20201028/f30277ae/attachment-0001.htm>


More information about the webkit-unassigned mailing list