2009/12/10 John Abd-El-Malek <[email protected]> > > On Thu, Dec 10, 2009 at 1:22 PM, Evan Stade <[email protected]> wrote: > >> On Thu, Dec 10, 2009 at 10:58 AM, Peter Kasting <[email protected]>wrote: >> >>> On Thu, Dec 10, 2009 at 10:45 AM, Jonathan Dixon <[email protected]>wrote: >>> >>>> In essence: >>>> >>>> return DoWork(&foo) >>>> #if defined(OS_POSIX) >>>> && DoWork(&posix_specific) >>>> #endif >>>> ; // <-- Lint complains about this guy >>>> >>> >>> I'd prefer this: >>> >>> #if defined(OS_POSIX) >>> return DoWork(&foo) && DoWork(&posix_specific); >>> #else >>> return DoWork(&foo); >>> #endif >>> >>> The same number of lines, but much easier to read. >>> >> >> disagree. It's harder to read because it's not immediately obvious that >> some of the code overlaps. Scott's solution seems best to me. >> > > +1 Scott's solution seems best for me. The problem with the above solution > is that it contains code duplication. For DoWork(&foo), that might seem > small, but as parameters get added, functions get renamed, etc, it's more > work (and error prone) to update two locations instead of one. > > >> > Opps sorry for delay in following up (I'm still tuning my filters to cope with @chromium)
Scott's solution was also the first thought of, so very happy to go with that instead :-) What stopped me proposing this in the CL was I had noticed some nearby code does it via multiple returns, so I had attempted to keep consistent with the pattern:- if (!DoWork(&foo)) return false; #if defined(OS_POSIX) if (!DoWork(&posix_specific)) return false; #end return true; Quite agree this doesn't look so obvious though. Cheers! -- Chromium Developers mailing list: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
