Skip to main content
Software Engineering

Return to Answer

added 690 characters in body
Source Link
Flater
Flater
  • 56.3k
  • 8
  • 106
  • 157

If Clamper is in another assembly altogether, then it does need unit testing as it is public. But your question makes it unclear if this is relevant.

Sidenote
I'm not going to go into a whole IoC sermon here. Some hidden dependencies are acceptable when they are pure (i.e. stateless) and don't need to be mocked - e.g. no one is really enforcing that .NET's Math class be injected, and your Clamper is functionally no different from Math.
I'm sure that others will disagree and take the "inject everything" approach. I'm not disagreeing that it can be done, but it's not the focus of this answer as it's not pertinent to the posted question, in my opinion.

If Clamper is in another assembly altogether, then it does need unit testing as it is public. But your question makes it unclear if this is relevant.

Sidenote
I'm not going to go into a whole IoC sermon here. Some hidden dependencies are acceptable when they are pure (i.e. stateless) and don't need to be mocked - e.g. no one is really enforcing that .NET's Math class be injected, and your Clamper is functionally no different from Math.
I'm sure that others will disagree and take the "inject everything" approach. I'm not disagreeing that it can be done, but it's not the focus of this answer as it's not pertinent to the posted question, in my opinion.

Source Link
Flater
Flater
  • 56.3k
  • 8
  • 106
  • 157

Testing before and after

In TDD, should I add unit tests to refactored code?

"refactored code" implies you are adding the tests after you've refactored. This is missing the point of testing your changes. TDD very much relies on testing before and after implementing/refactoring/fixing code.

  • If you can prove that the unit test outcomes are the same before and after your refactoring, you've proven that the refactoring did not change the behavior.
  • If your tests went from failing (before) to passing (after), you've proven that your implementations/fixes have solved the issue at hand.

You shouldn't be adding your unit tests after you refactor, but rather before (assuming these tests are warranted of course).

Refactoring means unchanged behavior

Should new unit test cases be written for refactored code?

The very definition of refactoring is to change the code without changing its behavior.

Refactoring is a disciplined technique for restructuring an existing body of code, altering its internal structure without changing its external behavior.

As unit tests are written specifically to test the behavior, it doesn't make sense for you to require additional unit tests after refactoring.

  • If these new tests are relevant, then they were already relevant before the refactoring.
  • If these new tests are not relevant, then they are obviously not needed.
  • If these new tests were not relevant, but are now, then your refactoring must invariably have changed the behavior, which means you've done more than just refactoring.

Refactoring can inherently never lead to needing additional unit tests that were not needed before.

Adding tests needs to happen sometimes

That being said, if there were tests that you should have had from the beginning but you had forgotten it until now, of course you can add them. Don't take my answer to mean that you can't add tests just because you had forgotten to write them before.

Similarly, sometimes you forget to cover a case and it only becomes apparent after you've encountered a bug. It's good practice to then write a new test that now checks for this problem case.

Unit testing other things

Should unit tests be added to the class Clamper?

It seems to me that Clamper should be an internal class, as it is a hidden dependency of your HPContainer. The consumer of your HPContainer class doesn't know that Clamper exists, and doesn't need to know that.

Unit tests only focus on external (public) behavior to consumers. As Clamper should be internal, it requires no unit tests.

Clamping?

I don't think the clamping method is all that necessary to begin with.

public static int ClampToNonNegative(int value)
{
    if(value < 0)
        return 0;
    return value;
}

What you've written here is a more limited version of the existing Math.Max() method. Every usage:

this.currentHP = Clamper.ClampToNonNegative(this.currentHP - value);

can be replaced by Math.Max:

this.currentHP = Math.Max(this.currentHP - value, 0);

If your method is nothing but a wrapper around a single existing method, it becomes pointless to have it.