Is the C# internal keyword a code smell?

In this post, I am going to show why I think the internal keyword, when put on class members, is a code smell and suggest better alternatives.

What is the internal keyword?

In C# the internal keyword can be used on a class or its members. It is one of the C# access modifiers. Internal types or members are accessible only within files in the same assembly. (C# internal keyword documentation).

Why we need the internal keyword?

“A common use of internal access is in component-based development because it enables a group of components to cooperate in a private manner without being exposed to the rest of the application code. For example, a framework for building graphical user interfaces could provide Control and Form classes that cooperate by using members with internal access. Since these members are internal, they are not exposed to code that is using the framework.” (C# internal keyword documentation)

These are the use cases I saw for using the internal keyword on a class member:

Call a class’s private function within the same assembly.
In order to test a private function, you can mark it as internal and exposed the dll to the test DLL via InternalsVisibleTo.

Both cases can be viewed as a code smell, saying that this private function should be public.

Let's see some examples

Here is a simple example. a function in one class wants to access a private function of another class.

class A{
	public void func1(){
		func2();
	}
	private void func2(){}
}

class B{
	public void func(A a){
		a.func2(); //Compilation error 'A.func2()' is inaccessible due to its protection level
	}
}

The solution is simple — just mark A::func2 as public.

Let's look at a bit more complex example:

public class A{
	public void func1(){}
	private void func2(B b){}
}

internal class B{
	public void func3(A a){		
		a.func2(this); //Compilation error 'A.func2(B)' is inaccessible due to its protection level
	}
}

What’s the problem? just mark func2 as public as we did before.

public class A{
	public void func1(){ ... }
	public void func2(B b){ ...} // Compilation error: Inconsistent accessibility: parameter type 'B' is less accessible than method 'A.func2(B)' 
}

internal class B{
	public void func3(A a){		
		a.func2(this); 
	}
}

But we can’t ?. B is an internal class so it cannot be part of the signature of a public function of a public class.

Those are the solutions I found, ordered by easiness:

Mark the function with the internal keyword

public class A{
	public void func1(){ }
	internal void func2(B b){}
}

internal class B{
	public void func3(A a){		
		a.func2(this); 
	}
	
}

2. Create an internal interface

internal interface IA2{
	void func2(B b);
}

public class A:IA2{
	public void func1(){
		var b = new B();
		b.func3(this);
	}
	void IA2.func2(B b){} //implement IA2 explicitly because func2 can't be public
}

internal class B{
	public void func3(A a){ 
		((IA2)a).func2(this); //use interface instead of class to access func2	
	}
	
}

3. Extract A.func2 to another internal class and use it instead of A.func2.

internal class C{
	public void func2(B b){
	 //extract A:func2 to here
	}
}

public class A{
	public void func1(){}
	private void func2(B b){
		new C().func2(b); 
	}
}

internal class B{
	public void func3(){	//a is no longer needed	
		new C().func2(this); //use internal class instead of private function
	}
	
}

4. Decouple the function from internal classes and make it public. This is much dependant on what the function is doing with its inputs. decouple the internal classes can be very easy, very hard and even impossible (without ruing the design).

But we don’t have public classes we use interfaces…

Let's look at some more real-world example:

public interface IA{
	void func1();
}

internal class A : IA {
	public void func1(){}
	private void func2(B b){}
}

internal class B{
	public void func3(IA a){		
		a.func2(this);  //Compilation error IA' does not contain a definition for 'func2' and no extension method 'func2' accepting a first argument of type 'IA' could be found

	}	
}

Let's see how the previous solutions are adapted to this example:

Mark function with Internal. this means you will need to cast to class in order to call the function so This will work only if class A is the only one that implements the interface, meaning IA is not mocked in tests and there isn’t another production class that implements IA.

public interface IA{
	void func1();
}

internal class A : IA {
	public void func1(){}
	internal void func2(B b){}
}

internal class B{
	public void func3(IA a){		
		((A)a).func2(this); //cast to A in order to accses func2

	}	
}

2. Create an internal interface that extends the public interface.

internal interface IExtendedA : IA{
	void func2(B b);
}

public interface IA{
	void func1();
}

internal class A : IExtendedA {
	public void func1(){}
	public void func2(B b){}
}

internal class B{
	public void func3(IExtendedA a){		
		a.func2(this);

	}	
}

3. Extract A.func2 to another internal class and use it instead of A.func2.

4. Decouple the function from internal classes and add it to the public interface.

We can see that the internal keyword is the easiest solution, but there are other solutions using the traditional building blocks of OOP: classes and interfaces. We can see that the 2nd solution — adding an internal interface is not much harder than marking the function with the internal keyword.

Why not use the internal keyword?

As I showed in the previous examples, using the internal keyword is the easiest solution. But you are going to have a hard time in the future if you will need to:

Move the public class A to another DLL (since the internal keyword will no longer apply to the same dll)
Create another production class that implements IA
Mock IA in tests

You may think “But this is just one line of code, I or anyone else can change it easily if needed”. Now you have one line of code that looks like that:

((MyClass)a).internalFunction

but if others will need to call this function too this line will be copy-pasted around inside the DLL.

My Conclusion

I think marking a class member with the internal keyword is a code smell. In the examples I showed above it is the easiest solution, BUT can cause problems in the future. Creating an internal interface is almost as easy and more explicit.

Compare to C++

The C++ “friend” keyword is similar to the C# internal keyword. It allows a class or a function to access private members of a class. The difference is it allows access to specific class or function and not all the classes in the same DLL. In my opinion, this is a better solution than the C# internal keyword.