[webkit-reviews] review denied: [Bug 13592] parseMappedAttribute inconsistency : [Attachment 14350] First attempt

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat May 5 03:58:35 PDT 2007

Oliver Hunt <oliver at apple.com> has denied Rob Buis <rwlbuis at gmail.com>'s
request for review:
Bug 13592: parseMappedAttribute inconsistency

Attachment 14350: First  attempt

------- Additional Comments from Oliver Hunt <oliver at apple.com>

I'd lift add const String& value = attr-value() after the first name check.  It
seem icky to query multiple times.

I don't think this gains anything.

in those cases where you access attr->value() multiple times i think you should
just declare a var.  eg. the attributeTypeAttr case.

I'd consider a local const& to hold the name, but otherwise looks good

looks good

local for the attribute name, and maybe use locals in those branches that query
the value multiple times.

local for the name again

local for name, local for value on those branches that access the value
multiple times

local for the name

local for name, local for value those places it's used multiple times

ooh, good *and* fixed indenting

local for name if it's used multiple imes


local for value when it's used multiple times

More information about the webkit-reviews mailing list