0

I have several classes that looks like: (some of them have dozens of cases)

function __construct($value) {
    switch ($value){
        case "1":
            $value = Values::One;
            break;
        case "2":
            $value = Values::Two;
            break;
        case "3":
            $value = Values::Three;
            break;
        case "4":
            $value = Values::Four;
            break;
        default:
            $value = null;
    }

    parent::__construct($value);
}

And I'm already test the parent class, should I have test this class too? should I test every possible path? i.e:

assertEquals(Values::One, new Clazz(1));
assertEquals(Values::Two, new Clazz(2));
assertEquals(Values::Three, new Clazz(3));
assertEquals(Values::Four, new Clazz(4));

or do only one assertion? assertEquals(Values::One, new Clazz(1)); only

1

2 Answers 2

4

This kind of mapping logic is very easy to make mistakes in either when you initially write it or when an update is done.

You should have some test that verifies that the correct mapping is performed, including what happens if a value that can't be mapped is passed. If you have that as part of the tests for the parent class, then that is fine. Otherwise, you should create it for the current class.

If you have only a few values, like in the example you presented, then you might get away with it by just doing a review that all values are mapped correctly, but if you have a dozen or more values in a mapping, then it becomes very easy to overlook an error.

4
  • Thanks! I don't have such a validations in parent class so you recommend me test every case?
    – Michael
    Aug 9, 2017 at 14:50
  • May I suggest a code change and move the mapping to its own class, so the child can do something like this: parent::__construct(YourMapper::getValueFor($value));. Then the mapping can be tested separately from instantiating a class.
    – Kwebble
    Aug 9, 2017 at 19:58
  • @kwebble the mapping is all the purpose of this class, the parent just do few generic checks and put the value in datamember (that I can test)
    – Michael
    Aug 9, 2017 at 22:32
  • You only provided the constructor so to me it looks like a normal child class. I did not understand it only has a constructor. Knowing that, it acts more like a sort of factory.
    – Kwebble
    Aug 10, 2017 at 18:52
2

If you do testdriven development one working example and a non-working axample is enought.

If you want an automatted test (with full coverage) writing the test is much more work than the actual implementation and you can never be shure if your test cover all aspects: i.e. if you later introduce a Values::Fifteen do you have a failing test that reminds you that __construct(...) is incomplete?

For this type of problem it may be easier to write an integration test if you have an inverse function like this

foreach(int id : allIds) assertThat(id).equals(name2id(id2Name(id))

this way you donot know if id and name has the right representation but you can be shure that for every id there is a working conversion.

I use this technique to verify that object (de)serialisation is complete:

String jsonString = aTestJsonWhereEveryPropertyHasAVaule;
MyComplexObject myObject = json2MyComplexObject(jsonSting);
String  result = myComplexObject2Json(myObject);
assertThat(jsonString ).equals(result);

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.