2

Let's say that I have 2 public methods:

func didSelect(data: Data) {
    // do something

    self.view.showText(textForData(data))
}

func didDismiss(data: Data) {
    if data.isSomething {
        self.view.showText(textForData(data))
    }

    ...
}

private func textForData(data: Data): String {
    var text: String

    if data.distance == nil {
        text = "..."
    } else if data.distance < 1000 {
        text = "\(data.distance) m"
    } else {
        text = "\(data.distance / 1000) km"
    }

    return text
}

Both of them depend on the formatting logic of textForData.

textForData has (with this minimized implementation) 3 possible cases. If I do test every possible case for both of my public functions, I'll end up with 6 test methods, and 3 of them would also be testing the same logic that was already tested by the other 3.

What's the proper way of testing this?

Ps.: I could write a separate test for textForData and in the tests for the public methods I assert that the textForData is called, but that seems to break the encapsulation of my class and I don't want to make the testForData public. I also wouldn't like to make a separate class just for my textForData logic, because I would end up creating too many dependencies for this current class, and that logic doesn't seem to fit anywhere else besides in this class.

Rodrigo Ruiz
  • 4,248
  • 6
  • 43
  • 75
  • I have added an answer based on the feedback left on http://stackoverflow.com/a/7096107/102482. Hope that helps. – Finglas Apr 03 '15 at 10:37

2 Answers2

2

You have a few options here.

  1. Don't test textForData
  2. Duplicate the behaviour in each test of the public method that uses it
  3. Make textForData public
  4. Make a public class for textForData

Point 1 and 2 are undesirable.

You seem oddly against point 3, but there are benefits to doing this. You will be able to test this behaviour once, given you really care about doing so. I don't know Swift, but in other languages this isn't as bad as it seems. The general advice is to code against and interface, rather than an implementation. So the public interface of this class would have didSelect and didDismiss. Your production code would be expressed in terms of this interface, meaning even though textForData is a public method on the class you cannot access it directly.

The good news here is that your tests can be written against an implementation (in fact, they have to) so here you'll have access to all three methods. So you can test to your hearts content.

Point 4 is similar to point 3, but stored as a separate class. I'd opt for this given that you could argue we have broken the Single Responsibility Principle in point 3. To hide this I'd make a nested class to begin with given you state this code is only used within this one example. Again, your tests will have access to this using the same ideas as above.

You're code is moving towards embracing composition, therefore you should embrace the benefits such as small classes, well factored code and more.

Finglas
  • 15,518
  • 10
  • 56
  • 89
  • Are there any other options? If not, then I'll accept your answer. Even though the last one was one of my ideas presented in the question, your answer was the most detailed one, and will be helpful for other people. Thank you for taking the time to help me. – Rodrigo Ruiz Apr 05 '15 at 05:55
  • @RodrigoRuiz not anything else I can think of or use on a day to day basis. Pretty much one of those four options is what I would use for any code I had to write tests for. Context is key, so they are all valid it just depends on the scenario. No problem. – Finglas Apr 05 '15 at 09:23
  • Do you know anything about Clean Architecture from Robert Martin? If so, could you give me your opinion on this: http://stackoverflow.com/questions/29466292/clean-architecture-should-the-controller-talk-to-the-presenter – Rodrigo Ruiz Apr 06 '15 at 06:19
1

I think that the formatting of the data is an own responsibility. So you should extract it to its own class.

This class could be unit tested on its own.

Your other class(es) that use this one should be decoupled by using an interface instead of the class directly. The dependancy should be injected (for example in the constrcutor). You could write an default constructor creating the default class to simplify things in production code (poor mens dependency injection).

Then you could mock the formatter and test the other class(es) in isolation, verifying that the textForData method is called correctly.

Holger Thiemann
  • 1,042
  • 7
  • 17
  • Yeah, that was the last idea I had in my question, but the problem is that I'll end up creating too many dependencies for a very simple thing. It doesn't seem like this formatting logic should go outside this class, because that's the only thing this class does, receive calls, format data and send the formatted data to the view (i.e., Presenter). – Rodrigo Ruiz Apr 02 '15 at 17:28