Pendulum swing: internal by default by Mark Seemann
Declare new C# classes as internal by default, and public by choice.
This is an article in a small series of articles about personal pendulum swings. Here, I document a recent change of heart that's been a long way coming. In short, I now declare C# classes as internal unless they're driven by tests.
The code shown here is part of the sample code base that accompanies my book Code That Fits in Your Head.
Background #
When you create a new class in Visual Studio, the default accessibility is internal. In fact, Visual Studio's default templates don't add an access modifier at all, but if no access modifier is present, it implies internal.
When I started out programming C#, I don't recall thinking much about accessibility modifiers. By default, then, I'd be using mostly internal classes. What little I knew about encapsulation (information hiding, anyone?) led me to believe that the more internal my code was, the better encapsulation it had.
It's possible that I make my past self more ignorant than I actually was. It's almost twenty years ago: I don't recall all the details.
Public all the things #
When I discovered test-driven development (TDD) (circa 2003) all my classes became public. They had to. When tests are interacting with code in another library, they can only exercise the system under test (SUT) if they can reach it. The tests make the SUT classes public.
Yes, it's technically possible to test internal classes in .NET, but I don't believe that you should. I've yet to change my mind about that; no imminent pendulum swing there. You're testing something you care about. If the internal code serves any, any, purpose, it must be somehow observable. If so, verify that such observable behaviour takes place; if not, delete the code. (I'm sure you can dream up some corner cases where this doesn't hold; fine: I'm painting with a broad brush, here.)
For years, I applied TDD, but I wasn't aware of the red-green-refactor cycle. I rarely changed the public API that the tests interacted with, and when I did, I made sure to adjust the tests accordingly. If a refactoring gave rise to new classes, I'd often write tests for those new classes as well.
Imagine, for example, invoking the Extract Class refactoring. The new class would be as covered by tests as before the extraction, but what happens next is typically that you need to tweak it. When that happened to me, I'd typically write completely new tests to cover it. To do that, I'd need the extracted class to be public.
In this phase of my professional life, my classes were almost exclusively public, with internal classes only making a rare appearance.
One problem this tends to cause is that it makes code bases more brittle. Every type change is a potential breaking change. When every public class is covered by tests, this makes tests brittle.
I think that it's relevant to consider the context of the code base. At this phase of my professional life, I maintained AutoFixture, a fairly popular open-source library. I wanted that library to be stable so that users could trust it. I considered the test suite a guard of the contract. As long as a change didn't break any test, I considered it likely that it wasn't a breaking change. Thus, I was already conservative when it came to editing tests. I considered test to be append-only in principle.
I still consider it prudent to be conservative when it comes to a library with a public API. This doesn't mean, however, that this line of thinking carries over to code bases without a public (language-level) API. This may include web sites and services, but could also include installed apps. As long as there's no public API, there's no contract to break.
Internal by default #
In 2020 I wrote a REST API of middling complexity. I used outside-in TDD as a major driver. In the spirit of behaviour-driven development I favour describing the observable behaviour of the system. I use self-hosted state-based integration tests for this purpose. Only when I find that these tests get too complex do I grudgingly drop down to the unit-test level.
The things that I test with unit tests have to be public. This still leaves plenty of room for behaviour described by the integration tests to have internal implementation details. The code base I mentioned has several examples of that. Some of them I've already described here on the blog.
For example, notice that the LinksFilter shown here is an internal class. Its behaviour is covered by abundant integration tests, so I'm not afraid to refactor it if need be. Those LinkToYear, LinkToMonth, and LinkToDay extension methods that it uses are internal too.
Another example is the UrlIntegrityFilter seen here. The class itself is internal and its behaviour is composed from private helper functions. Its counterpart SigningUrlHelper is also internal. (Its companion SigningUrlHelperFactory, shown in the same article, is public, but that's an oversight on my part. It can easily be internal as well.) All that URL-signing behaviour is, again, covered by tests that verify the behaviour of the REST API.
Another example from the same code base can be found in its so-called calendar feature. The system is an online restaurant reservation system. It allows clients to browse a day, a month, or even a year to see if there are any free spots for a given time slot. You can see an example here. While I test-drove the calendar feature with integration tests, it quickly dawned on me that I had three disparate cases (day, month, year) that essentially represented the same concept: a period.
A period is a closed set of heterogeneous data. A year contains only a single datum: the year itself (e.g. 2021). A month contains both a month and a year, and so on. A closed set of heterogeneous data describes a sum type, and since I know that in object-oriented programming, sum types can be encoded as Visitors, I introduced a Visitor API:
internal interface IPeriod { T Accept<T>(IPeriodVisitor<T> visitor); } internal interface IPeriodVisitor<T> { T VisitYear(int year); T VisitMonth(int year, int month); T VisitDay(int year, int month, int day); }
I decided, however, to keep this API internal, since this isn't the only possible way to model this feature. As is the case with the other examples I've shown here, the behaviour is covered by integration tests. I feel free to refactor. In fact, this Visitor-based API is actually the result of a refactoring from something more ad hoc that I didn't like.
Here's one of the three IPeriod implementation, in case you're curious:
internal sealed class Month : IPeriod { private readonly int year; private readonly int month; public Month(int year, int month) { this.year = year; this.month = month; } public T Accept<T>(IPeriodVisitor<T> visitor) { return visitor.VisitMonth(year, month); } public override bool Equals(object? obj) { return obj is Month month && year == month.year && this.month == month.month; } public override int GetHashCode() { return HashCode.Combine(year, month); } }
This class, too, is internal, as are its two companions Day and Year. I'll leave it as an exercise for the interested reader to implement these two classes, as well as IPeriodVisitor<T> implementations that return the next or previous period, or the first or last tick of the period, etcetera.
Public by choice #
This shifted emphasis of mine isn't a return to a simpler time. It's not internal all the things! It's about shifting the default for classes that are not driven by tests. Those classes that are artefacts of TDD are still public since I don't directly unit test internal classes.
Other classes may start out as internal and then get promoted to public by choice. For example, I'd introduced a Seating class in the code base to model how long a seating was supposed to take:
internal sealed class Seating { internal Seating(TimeSpan seatingDuration, Reservation reservation) { SeatingDuration = seatingDuration; Reservation = reservation; } // Members follow...
Some restaurants have second seatings (or more). They give you a predefined duration after which you're supposed to be done so that they can reuse your table for another party. I'd used the Seating class to encapsulate some logic related to that, such as the Overlaps method:
internal DateTime Start { get { return Reservation.At; } } internal DateTime End { get { return Start + SeatingDuration; } } internal bool Overlaps(Reservation other) { var otherSeating = new Seating(SeatingDuration, other); return Start < otherSeating.End && otherSeating.Start < End; }
While I considered this a well-designed little class with good encapsulation, I kept it internal simply because there was no need to make it public. It was indirectly covered by test cases, but it was a result of a refactoring and not directly test-driven.
As I started to add a new feature, I realised that I'd be able to write new unit tests in a better way if I could reuse Seating and a variation of its Overlaps method. I considered it carefully and decided to make the class and its members public:
public sealed class Seating { public Seating(TimeSpan seatingDuration, Reservation reservation) { SeatingDuration = seatingDuration; Reservation = reservation; } // Members follow...
I made this decision after explicit deliberation. It didn't take long, though, but I did shortly stop to consider whether this seemed like a good idea. This code base isn't a reusable library in the wild, so I wasn't concerned about misuse of the API. I did consider, on the other hand, how this would increase coupling between the tests and the production code base. It didn't take me long to decide that in this case, I was okay with that.
Seating had already existed as an internal class for some time and had proven useful and stable. Putting on my DDD hat, I also thought that Seating represented a proper domain concept.
Conclusion #
You can go back and forth on how you write code; which rules of thumb you apply. For many years, I favoured public classes. I think that I even, at one time, tweaked the Visual Studio templates to explicitly create new classes as public.
Now, I've changed my heuristic. Classes driven into existence by tests are public; they have to be. Other classes I now make internal by default, and public by choice.
This is going to be my rule until I change it.
