This is a long question with many arguments, and I've done my best to provide answers to all arguments. But I want to take a moment to summarize here.
You've reasoned about your stance deeply, and many of your observations are reasonably correct. However, the conclusions you then draw from those observations seemingly come out of left field and do not logically follow from the observations you made.
Most feedback points I wrote generally boil down to that same feedback.
Ignoring Private Methods during Tests
Well-written integration tests could cover all cover paths of all units, thus testing units is never strictly necessary. Yet unit tests still have value: They make it easier to find the root-cause of a failure (as compared to integration tests, which typically require more hunting to find errors), and offer a more isolated environment in which to debug them (which is simpler due to having less moving parts, and potentially confounding issues).
I'm not quite following here. You say unit testing isn't necessary, but then give an excellent benefit it provides when you do it. If getting the benefit isn't enough of a reason to consider doing it, then how are you defining "necessary"?
Because by that logic, nothing is necessary when you don't want it. You didn't need to become a software developer. You didn't need to post this question. I didn't need to read your question. I didn't need to get out of bed today. Etc...
Using that same logic, testing isn't necessary. Just run the application and see that it works, right? I'll spare you the lecture on why that's not a good idea. You touched on the benefits of testing yourself so there's no point repeating it. I asssume we agree that testing is not strictly necessary to deliver software but really really really advisable if you want to do your job well.
The main point that is often glossed over is that a test's purpose is to fail, not to pass. Don't get me wrong, a failed test implies a bug in your code, and bugs are not good to have. But when you already have a bug, having a failed test to raise that red flag for you is a good thing.
Think of it this way: when comparing two doctors who investigate the same 1000 patients, the better doctor will catch and diagnose more diseases than the other doctor. It's not good for patients to have more diseases but the doctor's diagnosis did not cause the disease, and while the disease may be bad, the diagnosis of that disease is a good thing (it provokes action to treat the disease).
Translating back: It's not good for codebases to have bugs, but the unit test's failure did not cause the bug, and while the bug may be bad, the unit test failure because of that bug is a good thing (it provokes action to fix the bug).
Well-written integration tests could cover all cover paths of all units
This is the mistake you're making here. You're only thinking of tests that pass. Yes, if a full-blown integration test passes, that implies that all of the integrated components themselves work (assuming you wrote the assertion logic for everything you care about)
But when it fails, which I want to remind you is the real purpose of a unit test, then your single combined integration test is much vaguer than it needs to be. It just tells you that something failed. That's about as useful as an exception without a stacktrace, exception type and message, i.e. not useful at all. What failed? Why did it fail? Is that the only thing that failed?
Not to mention that you're also blind to the "two wrongs coincidentally happen to make a right" type of bugs. E.g. if I write an integration test that reverses a string twice and compares the initial and resulting values, then that test would also pass if I never reversed the string at all.
This is of course a dramatically oversimplified example, but similar issues can exist where one component just happens to counteract the buggy behavior of another component.
You already addressed the benefits of using failed tests to debug so I won't repeat that part of it. Just be aware that you should think of the failures as much as the successes, if not more.
As an aside...
Whenever I point out potential bugs or how certain tests don't catch everything, I often get a response along the lines of "well they just needed to properly test for that and they would've caught the issue".
The answer here is simple: if you make assumptions based on a developer who writes perfect tests, then testing is irrelevant as such a perfect developer would never create a bug in the first place.
There's no way for any developer to know that a test suite is 100% complete. Developers simply write tests until they feel confident about the test results. If any bug pops up, the test suite gets expanded to cover for that bug in the future.
This means that the more complex a test becomes, the more likely you are to forget to test one of its many possible outcomes. This is why integration tests should not be your only line of defense: too complex to reasonably guarantee that every interaction or chain of events is tested.
Unit tests also have a cost: they bind more strongly to more of the public interfaces of objects in your prod code than integration tests do.
Since unit tests are tests of a unit's public contract, your observation is tautological. Of course your tests need to change when your tested object changes. This comes in two flavors:
- Changes to the logic will require you to add/update/remove unit tests to account for that new/updated/removed behavior.
- Changes to the contract without logic changes (= refactoring) will require the unit test to be refactored without changing the purpose of the tests themselves.
That's just unavoidable. And I'm not quite sure how ro why you'r asserting that this is a relevant excuse to skip writing unit tests. That's like saying you don't need to brush your teeth because combing your hair takes less time/effort.
The observation ("I comb my hair faster than I brush my teeth") might be correct but the conclusion ("Brushing my teeth can be skipped since it's less efficient") you're drawing from it is wrong.
While "public interface" is usually assumed to be synonymous for "rarely-changing," that's just not true.
Same reply here: your observation isn't wrong but the conclusion you're drawing from it ("we should skip unit tests") is wrong.
Removing Private Methods from the Class under Test
how a class with too many private methods is indicative that perhaps it's doing too much
That's a straightforward application of SRP and it is correct.
Note, though, that this is just a suggestion, i.e. one (or many) possible indications of there being an issue in your class. There is no set amount of private methods which guarantee that SRP is being violated and that would force you to abstract.
There is a strong tendency for SRP-violating classes to have a lot of varied distinct logic, which could be written in a monolithic function or could be subdivided into private logic. In either case, SRP is violated. Don't read too much into the "private method" part of the suggestion itself.
I find that following this advice leads me far into the scary IAbstractMetaFactoryStrategyProvider land. It's a scary land, where all the verbs are nouned in senseless ways. Naming things is hard, the names tend to (almost necessarily) become more long/descriptive (because there's so many classes in such a system, and they all need to be easily differentiated from each other), and evermore abstract (i.e. less related to the business entities).
How were you able to name these supposed private methods, and then struggle naming the classes which replace the private methods? Both of them needed a name that describes their function, so I don't get why you supposedly struggle with the latter and not the former.
Also, nomenclature requires some skill/experience on the one hand (because it helps you separate the correct distinct responsibilities), but also requires you to understand the problem you're trying to solve.
If you're struggling to come up with a name for some piece of logic that you clearly just decided needs to live on its own in its own class, then I think you're either abstracting the wrong thing or you haven't understood your abstraction well enough to understand its purpose (and thus the name it should receive based on that purpose).
With your MacOS example, your conclusions go off the rails. I want to address each point separately:
plistName(forDomain:) function is pretty niche, so I made it private.
...but not reusable enough to warrant being extracted out
That is not a reason for making it private. Read up on SRP and notice how a class' responsibility is not defined by how often it is used.
It it's a separate reponsibility, even if only containing a few lines of code, that can still justify making a separate class. For example, you could have different classes for generating the plist file path, accessing the plist file on a given path, and parsing the data from that file on the given path.
Regardles of whether these three responsibilities require many or few lines, the three responsibilities are distinct and will generally warrant abstraction.
It's not terribly complex, but imagine it was complex enough to warrant being tested on its own
Following on from the previous point, the size of a class does not define whether it should be tested or not. Anything that's either custom business logic or in any way non-trivial or prone to breaking should be tested.
You clearly have specific business rules for this plist path generation (NSGlobalDomain vs default), and this already proves the necessity of some tests to see if the right path (NSGlobalDomain vs default) is being generated under the right conditions (the input string).
I would just go ahead and unit test this private method, and call it a day.
Unit testing private logic is not a thing. Unit tests focus on the public contract. Private logic is an implementation detail, and tests explicitly avoid implementation details (except the mocking of dependencies).
Your test is an outside consumer. It only has access to the public methods/properties of your class, not the private ones. The consumers of this class (the test, production code, ...) do not care, nor do they need to know if your class has private logic or not.
If that private logic is triggered when a certain public method is called, then the unit test for that public method will pick up on any bugs generated by the private logic.
If there is no public method/property which triggers this internal logic, then this logic isn't being used, and therefore can be removed.
In the end, all of your class' content should be part of its public behavior. If something isn't part of the public behavior, then it can safely be deleted since it's clearly not being used.
And I mean, fair enough, but I'm not scared of refactoring.
First of all, "I"m not scared of X" is not the same as "I should do X". I'm not scared of breaking glass but that doesn't mean I should break my windows.
Secondly, if you're not scared of refactoring, why were you arguing that unit tests should be skipped because of the bother of having to refactor them when your public contract changes? You can't logically argue both points at the same time.
If this function/class was well written, i.e. it doesn't have many dependencies (it doesn't "pull in the world"), then extracting this method is absolutely trivial.
We're back at the aside comment I made. If you already assume that things were being done properly at all times, then the code shouldn't need refactoring to begin with since it would've been perfect when it was first written.
Secondly, the amount of dependencies of a class is not a guiding compass for when a chunk of private logic should be abstracted or not. The correctness of an abstraction is not defined by how much effort it takes to refactor your old code.
I feel no need to take pro-active measures against this refactoring in the future, because really, it's trivial.
It's a very common biased mistake for developers to make decisions about something they currently know a lot about (becayse they're handling it).
What you're forgetting is that it's not going to be trivial several months down the line, or when another developer has to deal with your code which he has never seen before, or when many other classes were added and the codebase itself is complex (even if the classes themselves are relatively simple), or when there's many consumers of your logic who depend on the public contract that you now need to refactor (thus leading to breaking changes).
All of these factors further compound the difficulty of having to refactor your classes. It's not hard to go around a tree, but it is hard to go through a forest. You may think you're only planting one tree today but if everyone plants trees thinking "it's just one tree", soon you'll have a forest that no one can easily cross anymore.
That's a LOT of boilerplate/overhead, that increases complexity/noise of the system, with no perceivable value if it's done "too early".
YAGNI certainly has its purpose. But YAGNI focuses on premature optimizations and overengineering based on current features.
That's not what you're talking about. What you're talking about is refusing to implement an abstraction that has already proven to be relevant, on the basis that it's a small change, you can live with the less clean implementation, and thus you are going to put this off for as long as possible.
The same response applies that I give to junior developers: if it's such a small change, why are you spending such a long time arguing just to avoid doing it? If it's a small change, just do it now and be done with it, less to keep track of. If it's a big change, then we really should look at it now because we shouldn't waste time in the future on analyzing the thing we already know and understand today.
It's 10s of extra lines, for a helper function that's only 5 lines long (and could conceivably be pretty well-written as a perfectly understandable, single-line conditional expression domain == "NSGlobalDomain" ? ".GlobalPreferences.plist" : domain + ".plist")
I really want to stress this point:
LINE COUNT IS NOT A VALID MEASURE OF CODE QUALITY
What matters is the logic, its branching complexities and its distinct responsibilities.
Secondly, condensing your logic into a single line is shorter, but is not always the most readable choice. Take it from me, someone who has written some dirty ternary operator chains (3 or 4 levels deep) in my days as a junior developer.
Do you know when I stopped doing things like that? The first time I had to revisit a project 6 months after having dropped it. Suddenly, I understood how code that makes sense when you write it (fully understanding the goal) is not necessarily readable in the future (when you're trying to refresh your memory on what its goal was).
Code is read more often than it is written. Therefore, we should prioritize the ease of reading the code over the ease of writing it. If a few extra lines means that it's much easier to understand by a developer who doesn't yet closely know the domain, then that's most definitely worth doing.
I would find myself extracting this code for the wrong reason: not because I have any intent to reuse this helper function,
Abstraction certainly promotes code reuse, but reuse is not the only reason to abstract logic.
"I want to reuse this, so I should abstract it" is correct. "This should not be abstracted since I don't want to reuse it" is not correct. The latter is not logically consistent with the former.
Choosing to Test a Private Method
But extracting this private method would make no difference to its stability. Public or private, if I need to change it, I will (and adjust the tests accordingly).
You're correct about the change needing to happen in either case. However, you're only thinking of the optimistic outcome of your refactoring working without additional side effects.
What happens if you happen to introduce a bug? Since you argued against unit tests and against additional abstraction, depending on the consequence of that bug you're going to either not see it, or your integration test is going to give you a vague failure that doesn't pinpoint the issue.
And that's assuming the issue is immediately visible when implementing the refactoring. If this issue remains hidden for some time, then you're going to spend a lot of time figuring out what went wrong.
If you had separated your responsibilities into separate classes, and unit tested each of them, you would've caught the issue earlier, and with more information (i.e. you'd know exactly which public behavior of which component failed).
Your actual questions
- Have I misunderstood about these principles, their motivations, and costs vs benefits?
Like I said before, you're making some correct observations but they aren't actually justifying the conclusions you seem to draw from them.
There's an underlying tone in your question where you're already aware of the benefits in doing something, but your argument often boils down to "that's extra work" or "I can do that at a later time". No offense, we all encounter it in our personal careers, but that's laziness talking.
- How should I handle this specific plistName(forDomain:) example?
Whether the path generation and file content retrieval should be separated right now or not is not conclusively answerable. There are many factors that can change this:
- Agile: generally favors keeping things simple until that implementation is no longer adequate
- Waterfall: generally favors doing things the right way from the start
- If these files are generated by an external resource, then your path format is a business requirement, and the generation of the correct path should be separated logic that can be tested directly
- If these files are generated and consumed internally in your application and amount to nothing but an implementation detail, then testing it becomes more a matter of due diligence as opposed to business requirement. I still advise testing it separately but the need to do so is lower than in the previous bullet point.
- Are you reasonably expecting other possible path formats to be included? ("reasonably" is an important modifier here!)
- Is this path format liable to change (e.g. macOS update)?
You need to weigh your options carefully here. But how you implement it doesn't change whether and how to unit test it. Unit tests don't care about implementation, only public contract. The public contract (string input, path output) seems pretty rigid in your case. But even if it wasn't, that's not an argument against unit testing.
- Does the guidance here change depending on whether or not this is a hobby personal project, or a professional team project?
That depends what you're asking. Are you asking...
- ... what makes you more employable?
- ... what minimizes risk of project failure or budget overruns?
- ... what maximizes code quality?
- ... what maximizes the ability for different developers to work in this codebase?
- ... what minimizes regressions or uncaught mistakes?
- ... what minimizes the time to deliver the project?
- ... what the cleanest theoretical approach is barring practical considerations?
- ... what would be expected of a junior developer?
- ... what would be expected of a senior developer?
Different focus, different answer.
For example, pure CompSci theory means the context (professional/hobby) is wholly irrelevant since it focuses on algorithmic elegance/performance.
Another example, your company may focus on a strict budget more than the quality of the final product, or vice versa. I can't answer that, that's completely contextual.
I.e. would being "strict" offer benefits to a single dev working on a single project, or are they more to do with large software systems that evolve more.
Again, context is key. Even if something doesn't net you a direct benefit in a personal atmosphere, it might still be good practice/training experience to then apply that in a professional context.
I personally cut particular corners in my personal projects that I don't cute professionally. For example, I assume that my codebase will never move away from EF/MS SQL Server. Why? Because I haven't done so in 10 years, have no intention to, and my personal time is quite limited to I want to maximize my output instead of keeping my projects in limbo.
But I would never do so for a client (unless explicitly instructed), since the cost of making a wrong assumption is massively larger than for my personal pet projects.