Internal Classes and Members

What’s the use for the internal access modifier in C#? It’s not as common to use as public, protected or private. It wasn’t until I started some serious stand alone library work that I fully started to appreciate and use internal.

All was fine and I was happy with my use of internal. Until I saw this tweet yesterday.

That tweet by Mark sparked an interesting discussion, but as great as Twitter is for new ideas, I can’t fit my thoughts on this subject into the 160 chars limit. Actually that last sentence alone is too big for a tweet.

In one of my replies to Mark, I mentioned that I think that there’s a huge difference between using internal on an entire type and using it on specific members.

internal Types

When writing a library, there are always some utility classes or helpers that are useful for the library but not part of the public API. I think that those are perfect to make internal. That doesn’t mean that they shouldn’t be documented, nor that they shouldn’t be maintained. But it means that they are not part of the public API contract. I can make breaking changes to them without having to worry about compatibility or versioning issues. I got some support for that opinion.

Another use for internal is to introduce new functionality gently. Lately I’ve found that when I first introduce new functionality I make the new classes internal and only exposes the functionality through higher level abstractions/classes. Later, when the feature and code has been stabilized and isn’t as prone to changes any more, I make those classes public. It’s a way to start supporting with a new feature, without having a big API cut in stone right from the start.

internal Members

In the same way new functionality can be added to existing types by first adding it as an internal method which is later made public.

There is also another usage of internal: to hide part of a class’ functionality from outside users. The System.Web.HttpCookie class is a perfect example of this. Looking at it’s public signature, it looks like a simple data transfer object class. Basically just a bunch of properties and no behaviour.

public sealed class HttpCookie
{
  public HttpCookie(string name);
  public HttpCookie(string name, string value);
  public string Domain { get; set; }
  public DateTime Expires { get; set; }
  public bool HasKeys { get; }
  public bool HttpOnly { get; set; }
  public string Name { get; set; }
  public string Path { get; set; }
  public bool Secure { get; set; }
  public bool Shareable { get; set; }
  public string Value { get; set; }
  public NameValueCollection Values { get; }
  public string this[string key] { get; set; }
}

Looks simple enough. But there is an internal side to it as well. A side that can lead to very peculiar bugs.

internal bool Changed { get; set; }
internal bool Added { get; set; }
internal bool FromHeader { get; set; }
internal HttpResponseHeader GetSetCookieHeader(HttpContext context)

The three properties describe the cookie’s relation to the current http request. The cookie itself knows if it’s been added, was parsed from a header or has been changed. I think that these properties breaks the single responsibility principle – they should not be part of the cookie, they describe the cookie’s relation to the response and request. Thus that information should be kept in the entry in the response cookie collection. But now it’s kept in the cookie itself and it’s kept hidden from the public API. If one where to preconstruct cookie objects that are added to different responses as needed this would lead to very hard to track down bugs. This is a typical example of how to not use internal.

The GetSetCookieHeader() method makes sense to have on the HttpCookie method. But why is it internal? Why not allow users of the class to benefit from the functionality to serialize the cookie to the transport format?

Unit Testing

There’s a final case I want to mention too and it is unit testing. In rare cases it makes sense to allow direct creation of objects in particular states for unit testing purposes. In this case some property setters or constructors that in effect are private can be made internal. Together with an InternalsVisibleTo attribute for the unit test assembly the unit tests can get access to state that are otherwise considered internal to the class.

internal is a Code Smell

I agree with Mark here. internal is indeed a code smell. It should be used with careful consideration. Sometimes it’s appropriate, but often it isn’t.

1 comment

  1. It would have been nice with a more compelling example to defend the use of internal members. Alas, this post even itself admits that the example design “can lead to very peculiar bugs”.

    Indeed, that was my original point: this is a code smell. The HttpCookie class lacks encapsulation. Essentially, encapsulation as originally described by Bertrand Meyer is a set of pre- and postconditions that ensure that the object can’t get into an invalid state.

    If HttpCookie can get into an invalid state, it lacks encapsulation. It’s a code smell; it should trigger investigation and redesign. Only rarely is making members internal the best response.

Leave a comment

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.