1
\$\begingroup\$

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 through testM, I merely checked hard-coded values in an array
  • testII through testXVII, I looped over all the chars and added values together
  • testIV through testIIX required a conditional modification to intVal()
  • testXXXIX (to the end) took a complete rewrite of intVal() 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:

  1. Is this a good work flow for TDD? If not, what could I improve?
  2. Did I run enough tests? Do I need to write more?
  3. 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();
\$\endgroup\$
3
\$\begingroup\$

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

\$\endgroup\$
  • \$\begingroup\$ Thank you for your feedback. I agree about RMC. Good points about toDecimal() instead of intVal(). 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 like composering 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 why RMC. I was going for RomanNumeralsConverter and just wanted to save typing. So it should have been RNC lol. I could have done use RomanNumeralsConverter as RNC; to have the shorthand. \$\endgroup\$ – Reed Jul 6 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 Jul 6 at 20:35
  • \$\begingroup\$ I'm rambling a bit because I don't necessarily know what I'm talking about lol \$\endgroup\$ – Reed Jul 6 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 Jul 6 at 21:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.