[webkit-reviews] review requested: [Bug 36167] Spatial Navigation: Add isNull and document convenient methods to FocusCandidate : [Attachment 50933] patch 0.2 - addressed kov's suggestions

[bugzilla-daemon at webkit.org]

Antonio Gomes (tonikitoo) <tonikitoo at webkit.org> has asked  for review:
Bug 36167: Spatial Navigation: Add isNull and document convenient methods to
FocusCandidate
https://bugs.webkit.org/show_bug.cgi?id=36167

Attachment 50933: patch 0.2 - addressed kov's suggestions
https://bugs.webkit.org/attachment.cgi?id=50933&action=review

------- Additional Comments from Antonio Gomes (tonikitoo)
<tonikitoo at webkit.org>
thx for reviewing Gustavo.

> convenience methods sounds better.

Changed.

>> A followup refactory patch will be making use of these helper methods.
>refactoring

Changed.

> Having a default value in a constructor doesn't
> look very idiomatic to me. I may be wrong, given my
> small experience with C++, but then again, most code
> I've touched in WebKit have two separate constructors
> for this kind of case - one for when you don't want to
> give it a node, one for when you have to give it a node.

Done. I added an additional constructor as suggested, as well
as a private |init| method that assigns default values to all
other class member data commonly initiated by both class construtors.

> Otherwise, the patch is fine, but I think it would be
> preferable if we have at least some code refactored to
> use the new methods in the same patch. r- for the
> constructor problem.

bug 36168 is all about this new refactored code :-)