I just learned about Test Driven Development on a podcast yesterday. So I decided to try it out today by writing a roman numerals to integer converter (per their suggestion). I've never written Unit Tests before today.
My process
I wrote the test class with the runAll()
function, then I started writing my tests one at a time, from top to bottom, if you look at the RomanTest class below.
Between each chunk, I significantly changed the intVal()
method of RMC
.
testI
throughtestM
, I merely checked hard-coded values in an arraytestII
throughtestXVII
, I looped over all the chars and added values togethertestIV
throughtestIIX
required a conditional modification tointVal()
testXXXIX
(to the end) took a complete rewrite ofintVal()
leading to the code posted below
Before writing the next test, I always confirmed the previous test was passing. Except, I wrote testL
-testM
all at once as those were VERY simple after getting testI-textX
working.
At points, when re-writing intVal
, my older tests broke. Then I would get all my tests passing again before continuing to the next test.
I never altered any previous tests when trying to get the new tests passing.
My questions:
- Is this a good work flow for TDD? If not, what could I improve?
- Did I run enough tests? Do I need to write more?
- Is it okay that, during
intVal()
re-writes, previous tests broke? (considering I got them all passing again before moving to the next test)
I am asking for a review of my TDD code & process.
class RomanTest {
public function runAll(){
echo '<pre>'."\n";
$methods = get_class_methods($this);
foreach ($methods as $method){
if ($method=='runAll')continue;
$mo = $method;
while (strlen($mo)<15){
$mo .='-';
}
echo "<b>{$mo}:</b> ";
echo $this->$method() ? 'true' : 'false';
echo "<br>\n";
}
echo "\n</pre>";
}
public function testI(){
$rmc = new RMC("I");
if ($rmc->intVal()===1){
return TRUE;
}
return FALSE;
}
public function testV(){
$rmc = new RMC("V");
if ($rmc->intVal()===5){
return TRUE;
}
return FALSE;
}
public function testX(){
$rmc = new RMC("X");
if ($rmc->intVal()===10)return TRUE;
else return FALSE;
}
public function testL(){
$rmc = new RMC("L");
if ($rmc->intVal()===50)return TRUE;
return FALSE;
}
public function testC(){
$rmc = new RMC("C");
if ($rmc->intVal()===100)return TRUE;
return FALSE;
}
public function testD(){
$rmc = new RMC("D");
if ($rmc->intVal()===500)return TRUE;
return FALSE;
}
public function testM(){
$rmc = new RMC("M");
if ($rmc->intVal()===1000)return TRUE;
return FALSE;
}
public function testII(){
$rmc = new RMC("II");
if ($rmc->intVal()===2)return TRUE;
return FALSE;
}
public function testIII(){
$rmc = new RMC("III");
if ($rmc->intVal()===3)return TRUE;
return FALSE;
}
public function testVI(){
$rmc = new RMC("VI");
if ($rmc->intVal()===6)return TRUE;
return FALSE;
}
public function testVII(){
$rmc = new RMC("VII");
if ($rmc->intVal()===7)return TRUE;
return FALSE;
}
public function testXVII(){
$rmc = new RMC("XVII");
if ($rmc->intVal()===17)return TRUE;
return FALSE;
}
public function testIV(){
$rmc = new RMC("IV");
return ($rmc->intVal()===4);
}
public function testIIV(){
$rmc = new RMC("IIV");
return ($rmc->intVal()===3);
}
public function testIIX(){
$rmc = new RMC("IIX");
return ($rmc->intVal()===8);
}
public function testXXXIX(){
$rmc = new RMC("XXXIX");
return ($rmc->intVal()===39);
}
public function testXXXIIX(){
$rmc = new RMC("XXXIIX");
return ($rmc->intVal()===38);
}
public function testMMMCMXCIX(){
return (new RMC("MMMCMXCIX"))->intVal()===3999;
}
public function testMLXVI(){
return (new RMC("MLXVI"))->intVal()===1066;
}
}
class RMC {
private $numerals;
private $vals = [
'I' => 1,
'V' => 5,
'X' => 10,
'L' => 50,
'C' => 100,
'D' => 500,
'M' => 1000,
];
public function __construct($numerals){
$this->numerals = $numerals;
}
public function intVal(){
$tot = 0;
$lastTot = 0;
$lastVal = 0;
foreach (array_reverse(str_split($this->numerals)) as $nml){
$value = $this->vals[$nml];
if ($lastVal<=$value){
$tot +=$value;
$lastVal = $value;
} else if ($lastVal>$value){
$tot -= $value;
}
$lastTot = $tot;
}
return $tot;
}
}
And to run it:
$test = new RomanTest();
$test->runAll();
1 Answer 1
It is a good thing you completely seperated your testing code from the code you're testing.
All your tests have the same basic structure, so why not create an array containing:
[1 => "I",
2 => "II",
3 => "III",
4 => "IV",
5 => "V",
............];
And use that array to run your tests. You could even use the same array to test a function that does the inverse.
Moreso, having the inverse function is almost a must. It would further simplify the testing, since you can convert decimals to Roman numerals and back again. That way you don't need an array at all to do many tests.
Please pay attention to the names you choose. To me RMC
doesn't mean much. If you encounter this class name two years from now, will you immediately know what it means? I don't think so. Why not give it a proper name like: RomanNumerals
? This class could have two methods: toRoman()
and toDecimal()
. The intVal()
method name is confusing because there is already an intval() function which does something else.
Your questions
Is this a good work flow for TDD? If not, what could I improve?
You understand the basic principle, however I think it would have been possible to write all the tests before writing the real code. This is also often done in real life: The tests are known before the code is written. The writing of the code is driven by the tests. Or, to put it in another way: If you don't know what you're going to test, then how on earth could you write any code?
Did I run enough tests? Do I need to write more?
As I illustrated before, by writing the inverse function as well, you can do thousands of tests, without having to be very selective about it.
Is it okay that, during intVal() re-writes, previous tests broke?
Yes, that's fine. That's the whole idea behind test-driver-development: The tests tell you what's wrong.
Remember that there are frameworks for writing tests. It is clearly possible to work without them, but why reinvent the wheel again? See, for instance: PHPUnit
-
\$\begingroup\$ Thank you for your feedback. I agree about
RMC
. Good points abouttoDecimal()
instead ofintVal()
. Writing the inverse would make a lot of sense, and indeed would make it easy to run MANY tests. I'll give PHPUnit a go. Didn't feel likecomposer
ing it in for such a simple use case. But I'm sure it (or something similar) is what I should be using going forward. IDK whyRMC
. I was going forRomanNumeralsConverter
and just wanted to save typing. So it should have beenRNC
lol. I could have doneuse RomanNumeralsConverter as RNC;
to have the shorthand. \$\endgroup\$Reed– Reed2019年07月06日 20:30:16 +00:00Commented Jul 6, 2019 at 20:30 -
\$\begingroup\$ So do you think it is, generally, better to write ALL the tests before writing any code? I personally kind of like the incremental fashion of writing one test, failing, writing code to pass the test, then writing the next test, failing, coding, passing, repeat. Breaks the project down into smaller chunks of just building one tiny piece at a time. But I suppose I could write all the tests & then still focus on one test at a time if I need that bite-sized approach. \$\endgroup\$Reed– Reed2019年07月06日 20:35:41 +00:00Commented Jul 6, 2019 at 20:35
-
\$\begingroup\$ I'm rambling a bit because I don't necessarily know what I'm talking about lol \$\endgroup\$Reed– Reed2019年07月06日 20:36:12 +00:00Commented Jul 6, 2019 at 20:36
-
1\$\begingroup\$ Your incremental method clearly works, and it is still test-driven. For bigger projects it is probably the way to do it. \$\endgroup\$KIKO Software– KIKO Software2019年07月06日 21:55:11 +00:00Commented Jul 6, 2019 at 21:55