Being more explicit can be a bad thing, when repetition and unnecessary fluff obscure the really important things. That's why we have pronouns, for example. Of course it's subjective exactly where the line is.
For the record, I'm more tolerant than Nate of pointer comparisons to NULL, even though they are unnecessary and I'm happier not to use them. It's the explicit comparison of a boolean value to 'true' or 'false' (which I'd never seen before looking at Ruby code) that really bugs me, since to me it makes the code much less naturally readable (you don't do that in English) and largely defeats the purpose of having a boolean type. If you're going to do that, why don't you say "if ((x > 0) == true)" too? How do you know when to stop? Maybe "if (((x > 0) == true) != false)" would be better! Just teasing ;-) Steve On Thu, Mar 18, 2010 at 5:00 PM, Derek Hower <[email protected]> wrote: > Out of curiosity, what is the reasoning behind removing the > comparison? It seems that being more explicit isn't a bad thing. > > On Thu, Mar 18, 2010 at 5:47 PM, nathan binkert <[email protected]> wrote: >>> diff --git a/src/mem/ruby/system/DMASequencer.cc >>> b/src/mem/ruby/system/DMASequencer.cc >>> --- a/src/mem/ruby/system/DMASequencer.cc >>> +++ b/src/mem/ruby/system/DMASequencer.cc >>> @@ -96,7 +96,7 @@ >>> len : >>> RubySystem::getBlockSizeBytes() - offset; >>> >>> - if (write) { >>> + if (write && (data != NULL)) { >> >> I have to agree with Steve that this should be (write && data), but >> I'm just givin you a hard time you >> >> >>> + if (active_request.data != NULL) { >> and again >> >>> + if (active_request.data != NULL) { >> and again >> _______________________________________________ >> m5-dev mailing list >> [email protected] >> http://m5sim.org/mailman/listinfo/m5-dev >> > _______________________________________________ > m5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/m5-dev > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
