On Thu, Dec 10, 2009 at 7:57 AM, Joe Mason <jma...@rim.com> wrote: > > Off the top of my head as a reviewer I'd accept: > > > > if (color.red() == 255 && color.green() == 0 && color.blue() == > > 255) // pink! > > > > over > > > > if (color.red() == 255 && !color.green() && color.blue() == 255) > > // pink! > > > > most days of the week... Consistency and all that. > > The actual potential bug in this comes when "if (!color.green())" comes > on its own - it looks to a casual glance like green() returns a bool > saying whether this color is green or not.
But if "!color.green()" is potentially confusing, then certainly it is just as confusing (in fact moreso) without the surrounding "color.blue()" and "color.red()" tests in Adam's example. Yet Adam cited "consistency with surroundings" as the reason to prefer == 0 in this case, which suggests that he'd be fine with an isolated test of this value. My point is that your argument (and Adam's) is not actually an argument for reviewer discretion, or promoting consistency over whatever the style guide says; it's an argument that the style guide is wrong to prefer "!foo" over "foo == 0". PK
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
