3 ms·
> Private methods only exist to serve public methods. > So if you are testing all the public methods you are at the same time testing the private methods that
by jcelerier 5y ago
> Private methods only exist to serve public methods.
> So if you are testing all the public methods you are at the same time testing the private methods that get called.
that's nice in theory but in practice if your API looks like
class my_algorithm {
public:
float compute_next_value(float);
private:
void deborgle_fields(...);
void reticulate_splines(...);
void deboor_network_max_span(...);
vector<float> m_tempArray1, m_tempArray2, ...;
vector<complex<double>> m_protoscalarField[3];
};
you'll have a much better time if you can test every part of the algorithm individually, even if these steps are entirely specific to this very algorithm and useless to any other code and must not be mingled with with API users under any circumstance (and thus made private)
- Toine 5y agoChange your API then. In this case you usually have 2 classes : a low-level algorithm-specific one, used by a higher-level consumer-centric one. Most of the time, testing private methods is a poor solution to a larger design problem.
- iainmerrick 5y agoWhy expose the low-level algorithm-specific stuff in your public API at all, though? I don't think "just for testing purposes" is a good answer. Unit testing hooks shouldn't be in the public API of a library.
- ahoka 5y agoWell, don't expose them then?
- jesseduffield 5y agoI see this as the main tension in the extracting-class viewpoint. Assuming your language allows it, you can extract out a class that's still private to the current package, meaning that the public API is unchanged. But there are still costs to extracting out that class. If the one public method just calls private method A, B, and C in order, passing the result of one step to the next, and if we don't need to handle different variants of A, B, and C (meaning dependency injection is not necessary), we arguably don't gain much from splitting the original class in two. We're left with a very small class of a single method that's tightly coupled to the extracted class. We can remove the coupling with dependency injection, but now we need to add some code that actually injects that dependency at runtime. So understanding how the pieces fit together is harder. The only benefit to splitting the class that I can think of is in preparing for a future situation where orchestrating the three helper methods becomes more complex, or where you actually do need to handle different variants of A, B, and C via dependency injection. But it feels like premature abstraction to me, single responsibility principle notwithstanding.
- iainmerrick 5y agoI see this as the main tension in the extracting-class viewpoint. Assuming your language allows it, you can extract out a class that's still private to the current package, meaning that the public API is unchanged. But there are costs to extracting out that class. I don't see why the class needs to be extracted in that case. If it makes the code easier to follow, do it; if it adds bloat and boilerplate, don't do it. The key thing is the ability to expose stuff for testing only, not in the public API. Depending on the language, you might not need to extract a class at all -- just make some fields or methods package-private. I guess if your language doesn't provide a way of doing that, extracting a class is a reasonable workaround because you can segregate your public API into high-level and low-level parts, and recommend that end users avoid the low-level parts. But that's not ideal; better to hide the low-level stuff if you can.
- jesseduffield 5y agoTo be clear, I was responding to the combined argument of the above comments: that you should split the class but not expose the extracted class in the public API. I agree that there are cases where we don't need to extract the class, but am willing to be convinced otherwise.
- withinboredom 5y agoThis is also good for maintainability. Say you need to upgrade the algorithm but keep the old version around. You can create a new version of the algorithm specific parts without changing the public implementation and allow users to select which version to use; eventually changing the default. In a single class, this would be a nightmare.
- iainmerrick 5y ago"Nightmare" is a bit strong; that seems like a very normal and straightforward refactoring step... iff you avoided putting the low-level stuff into your public API.
- withinboredom 5y agoI’ve mostly see people do public_namespace.private_internal.MyClass so you can use it if you really want to… but you probably shouldn’t.
- jcelerier 5y agoAny API changes to this would have bigger drawbacks than just making the tests friends of this class.
- Leherenn 5y agoI call those "implementation tests", and I delete the tests once I have finished the implementation. From experience, what you care about in that case is to make sure you implemented the private functions correctly, because it might be some tricky algorithm or whatever. Tests with a lower granularity than the public API make a lot of sense to achieve that goal, it's pretty similar to TDD (though you do not necessarily have the write the tests before the code). On the other hand, in the vast majority of cases, the code, once written, will not be changed. If the public API is changed, then it's likely the internal implementation will be replaced, or changed. In any case, the "implementation tests" will break because the contract has changed. So I delete them, the maintenance cost is higher than the long term value they bring, and non-regression should be ensured by the public API tests anyway. Sometimes, this is wrong, and you will actually need to extend the code in such a way that keeping the "implementation tests" would have helped. I haven't found a good way there to store the test "just in case". You could for instance have a commit that adds them then a commit that removes them, but this is a lot of noise in the history, and the probability that anyone but you can find the deleted tests if they need them is pretty much 0.
- jcelerier 5y agoI find it beyond crazy that you'd go through all this complicated decision state machine when you could just keep the tests and mark them as friends of the class. Why would you ever remove a test ? You don't know when your platform will change things from under your feet, for instance going from x86 to arm, which may break them in subtle ways without you changing anything to your code. And then you're losing hours searching in your 10k commit history the code that had been removed.
- Leherenn 5y ago> Why would you ever remove a test ? Because the cost of building/running/maintaining and generally just having the test around is higher than the value I get from the test. And because I still have unit tests on the public function that should catch issues. Really, I just wrote them to help me during the dev part. Every extra line of code is debt. > And then you're losing hours searching in your 10k commit history the code that had been removed. You're right, I said it was a possibility, but I don't do that, it's not worth it. It has happened twice so far I think that I wished I hadn't removed the tests. I bit the bullet and rewrote them, which is okay because of how rare it is.
- galaxyLogic 5y agoI see your point and it's interesting to think why that is so. When I test the implementation I am basically writing tests for myself to help convince myself that the code is doing what I think it does. Whereas testing the public interface helps other people in the same manner. Note it is possible that a test of a private method might fail yet all public methods calling it would still work correctly according to their "spec".