Should I public method and test it instead of testing a caller

Question

I have a method called GetValueA() and GetValueB() which return string that in formatted

 public string GetValueA(){
     string a = service1.GetA();
     return Format(a);
 }

 public string GetValueB(){
     string b = service2.GetB();
     return Format(b)  
 }
 
 private Format(string unformatted){
     return unformatted.ReplaceSpace().PaddingZero()  // for example " 123" will be "00123"
 }

when I create unit test I create

 GivenA_WhenGetA_ReturnFormattedA() // with test data "123", " 123", " 12 3"
 GivenB_WhenGetB_ReturnFormattedB() // with test data "123", " 123", " 12 3"

from above it seems the same so if I public private Format()

 public class Formatter : IFormatter 
 {
      public string Format(string data);
 }

and create unit test

 GivenDataHaveSpace_WhenFormatString_ReturnDataWithNoSpace();
 GivenDataLessThanFiveDigits_WhenFormatString_ReturnDataWithPaddingZero();

and use it in GetValueA and GetValueB

 public string GetValueA(){
     string a = service1.GetA();
     return IFormatter.Format(a);
 }

 public string GetValueB(){
     string b = service2.GetB();
     return IFormatter.Format(b)  
 }

the question are

1. if I use IFormatter instead of private it looks like all responsibility is rely on IFormatter and testing

   GivenA_WhenGetA_ReturnFormattedA() // with test data "123", " 123", " 12 3"
   GivenB_WhenGetB_ReturnFormattedB() // with test data "123", " 123", " 12 3"
   

   will rely on mock of IFormatter which is act as the same as I set up.

   1.1 Should I keep this, to say these methods need two format data after get from service?

   1.2 or just remove them because when I have something the same with GetValueD, GetValueE, GetValueF, GetValueG will make this redundant.

2. Is it make sense if I go with private Format() method instead?

Answer 1

Your code looks fine.

I'll assume .ReplaceSpace() and .PaddingZero() are extensions methods that have their own unit tests. This is exactly what extension methods are for, simple behaviour that has no side effects.

If the formatting is specific to this service and the result is not parsed by another service in your domain, a private method is good enough. Only if you actually need both sides (for example this is a format that needs to be parsed by another of your services later) then I would consider writing a serializer/deserializer service pair and interface, so the responsibility for the format is in one place and not scattered across your project.

There is no point in having an extra formatter interface and service, if it's not used across other services and won't be switched out for another implementation.

In case your format becomes complicated, but still free of side effects, you could put it into it's own extension method and unit test that. Then use that extension method instead of your private method.

There are two schools of thought. One says something is good if it's as simple as it can be and still fulfils the requirements. Another says that it's good when it's as complicated as it can be and fulfil the requirements. I'm a believer in the first one. Don't add an interface, service and injection configuration if all you need is a simple, side-effect free method. Just add one. Done.

Answer 2

I generally try and isolate the "interesting" stuff from the "boring" stuff and focus on testing the interesting stuff. In your example (without any additional context), Format seems like the interesting stuff so it makes sense to pull out the Formatter and test that separately. Testing it through GetValueA and GetValueB means you will end up with duplicate tests for the things that the formatter does.

Whether or not you keep the other methods around depends on what they mean in the context of your system. If they represent meaningful concepts, you will probably want to write a higher-level (perhaps acceptance) test around them, but if they're just internal details, you can either keep them around as helper methods or choose to inline them.

Answer 3

 public string GetValueA(){
     string a = service1.GetA();
     return Format(a);
 }

 private Format(string unformatted){
     return unformatted.ReplaceSpace().PaddingZero()  // for example " 123" will be "00123"
 }

This looks to me like a parser - we're taking some general purpose representation of a message from service1 and creating our own local representation of that information.

Parsers, as a general pattern, have enough complexity that I prefer designs that allow you to test them without extra ceremony. Here, the extra ceremony includes providing a service1, or some reasonable substitute for it. It's not hard to imagine GetValueA changing in ways that would require even more ceremony, so I would extract the formatting code into its own testable model soon.

That done, does the Format need to be mockable? My guess is that it really doesn't. Your parser is a self contained, in memory only thing; using the "real" parser doesn't add significant overhead. James Shore's Testing Without Mocks might be a useful reference.

The edge where I might start thinking about mocks/substitutes is the point where we discover that we want to be able to re-use the code in GetValueA with different Format functions. Until then, YAGNI.

Answer 4

Basic mode:

Test all of the public methods on everything.

Advanced mode:

Test the application, or top most level of your code. Other tests are optional