That's not more readable to me. Now it's confusing. Your
log shows that the "login is ok for testuser" before it
checks to see if it's ok. I understand your reasoning
for moving it, but in that instance your now causing
problems with the log. If I go to look at the log, I'll
see that the login was ok, when it may or may not have
been.
"Do we really need a double test with two different
values? I’m partial to this."
So am I, but in most Test Frameworks this doesn't
require you to write a separate test (i.e. xUnit.net has
Theory/InlineData for that)
Michael:
putting it up there is not a FINAL solution. it is an
increment of the full solution. it is there to force me
to create a test that proves it should *not* be there,
for all the reasons you mention.
it *should* stand out like a sore thumb, but the only
way to cure that pain is with a test.
You actually wanted to reply to J.R. Garcia :)
My 2 cents: Now that you moved the logger to the top,
you can write another test that verfies the logger is
NOT called if the login is not OK - you've just
successfully added another failing test to your arsenal,
which you can then refactor with confidence.
Shorter? Yes
public bool IsLoginOk(string user, string password)
{
log.Write("login ok for user: testuser");
return m_users[user] != null && m_users[user] ==
pasword;
}
would have to switch it back when you want to just log
when login is ok though... but still. Shorter == yes :p
excellent advice, thanks a lot!
Roy:
Ok. Then we can completely agree. I think those 4
questions are great
You might want to consider discussing the "refactor" in
"red-green-refactor" to avoid confusing readers. After a
test passes the next step is to refactor to remove
duplication.
> (in tdd you can only change production code
functionality when you have a failing test)!
That's not at all true. Anytime all tests are passing
it's always OK to refactor to improve the design which
would include simplifying the solution.
I don't think your example was very good, had you done
the simplest thing that could possibly work, you'd never
have to go back and try to hard code something because
you'd have hard coded it immediately to pass the first
test.
DTSTTCPW is about making the smallest change possible to
make a failing test pass, that's all. It doesn't apply
to the refactoring stage where you're trying to make the
code cleaner or improve the design. It only applies to
the make the failing test pass stage.
What about using a randomly generated user name in the
unit test? Then you will be forced not to hard code the
user name.
Great post, good advice.
Thanks,
Lee
@Ramon
The didactic cause of this post is served well by the
example. The point is to help people (Roy's students
specifically) characterize "the simplest thing that
could possibly work".
@Roy
Do the questions you ask at the end of your post about
the efficacy of the new tests point us to refactoring
the original test, i.e.
Should_Log_After_Successful_Login, to enforce a strict
ordering of the expectations? This would then might
force a refactoring to e.g. a loggedin event. In truth
the original test doesn't really specify the behavior
properly, does it?