Chris said:
This got me curious, so I did a search through a current project from
work to find all occurrences of "== true". I found seven. Six of those
came from a contractor we had working for us who had a very verbose and
tedious coding style, but he wasn't really an idiot.
I'll have to take your word for that. But it still triggers my suspicion that
either the writer just doesn't "get" the concepts that he is working with, or
that he is sloppy in expression and/or thinking. In neither case do I want to
work with the resultant code. Perhaps "idiot" wasn't the best word to try to
pack those meanings into, but at least it conveys the gist.
OTOH, six explicit tests against true isn't many (assuming that the programmer
wasn't spectacularly unproductive ;-) so perhaps they were just aberrations
that crept in un-noticed, but which weren't typical. E.g. I find it easier to
imagine refactoring:
if (workToDo() == 0) { ...
into:
if (workIsFinished() == true) { ...
than just typing the second form in from scratch.
Alternatively, maybe the guy was a spectacularly good programmer, and really
used very, very, few conditionals. So he wanted to ensure that they were
visible. Not really a serious suggestion, but I do increasingly feel that
conditionals are symptoms of / contributors to unclean code (meaning
ill-factored, or under-designed, or hacky -- in short "lumpy").
if (model.getTrueFalseCorrectAnswer() == true)
{
trueButton.setSelected(true);
}
It's written that way because it reads better. "true" is a possible
correct answer for the question. The boolean expression doesn't
represent a condition that's either true or false. I wonder if you
object to that.
Interesting example.
Yes, I think I'd put an explicit == true here too. I'm assuming that the
context is that there are (or could easily have been) other similar methods
like getIntegerCorrectAnswer() and, perhaps even getStringCorrectAnswer(). The
boolean isn't really being used /as/ a boolean -- its job is to model a
separate concept in the application domain. In a similar way, you wouldn't be
comfortable doing arithmetic on the results of getIntegerCorrectAnswer().
I'm not sure how much I like the idea of "punning" with Boolean like this. It
obviously depends on your application (and is perhaps getting into the realm of
over-engineering), but it seems to me that code like
if (model.getTrueFalseCorrectAnswer() == Test.TRUE_ANSWER)
{
....
shows the meaning more clearly. Especially since I'd expect the domain of
getTrueFalseCorrectAnswer() to be included in that of the (hypothetical) method
getTrueFalseStudentResponse() which would presumably include NO_ANSWER (and
hence could not be modelled as Boolean).
Still, the explicit test in this case clearly doesn't reflect an insensitivity
to the meaning of the program -- quite the reverse -- so I think I'd give the
author extra brownie points...
(Which, if it were needed, goes to show how damaging /mechanical/ code
criticism, and tools, can be.)
-- chris
