Suppose we have a sealed internal class C with a member M intended to be accessed from throughout the assembly:
internal sealed class C
{
??? void M() { ... }
}
Should the accessibility modifier at ??? be internal or public?
First of all, let’s establish that there is no technical difference between the two. public in C# means “accessible to anyone who can see the class”; making a public member of an internal class does not make the member more accessible than making it internal would.
There are good arguments for both sides.
The pro-internal argument is that the method is effectively internal, and you want to be able to glance at a method declaration and know whether it can be called by external code, whether it needs to be documented, whether it needs to verify that its arguments are valid, and so on.
There are several pro-public arguments. First, let’s state the meanings of the different accessibility modifiers:
private: this member is an implementation detail of the typeprotected: this member is an implementation detail of the type hierarchyinternal: this member is an implementation detail of the assemblypublic: this member is not an implementation detail at all; it is the public surface
The question then is whether from the point of view of the author of the class, irrespective of the accessibility of the class, is the member logically a part of the public surface, or not? If it is, then make it public.
Another way to say that would be: suppose we marked the member as internal, and then decided to make the class public instead of internal. Would we have to change the member from internal to public in order for the class to function? If the answer is “yes” then it should have been public in the first place.
Finally, other aspects of the language design encourage you to think of public in this way. For example:
internal interface I
{
void M();
}
internal sealed class C : I
{
public void M() { ... }
}
Even though I and C are both internal, the language requires that M be public in order to implement I.M.
As you could probably tell, I am strongly in favour of the “public” option. However there are some members of the C# compiler team that are in the “internal” camp; it’s not an unreasonable position. My advice is to discuss the issue amongst your team, make a decision, and then stick to it. That way you’ll all be able to read and understand the intention of the code.
Further reading:
Fabulous adventures in coding 
“public” simply means: Not an implementation detail. Can call from the outside.
“private” means: Implementation detail.
“internal” is a hack for situations where you *need* something in between. Internal class members should be rare because most members should be either implementation details or part of the public contract.
By making everything internal and nothing private you throw away this valuable piece of documentation and information hiding. Soon, internal callers will start to call into implementation details.
I disagree. Public is public to everyone, a published API or a re-usable class. Internal is for classes that have no use outside the assembly, which is most of the time.
I find more of my classes are internal than public, often. As for methods, I haven’t counted but internal is still common.
My philosophy falls in-between. I define most types as internal yet almost never define members as internal.
I think perhaps he meant _methods_, not _classes_. I think internal classes would be common, but internal methods should not, for the reasons he stated.
Indeed! I was unclear on that. All statements I made apply only to methods.
Of course, classes are often internal by necessity of hiding them from the outside. C# does not allow for more granular dependency management such as restricting what namespace can use what other namespaces. You can’t have “sub-assemblies” for intra-project dependency management. Need tools for that. If this was built-in I would often hide classes from other logical components of my app.
OK, I see. Well, in the past I have often followed the practice of “if I can restrict it further, I probably should”, so often the methods of my internal classes have themselves been internal.
As I said below, I think there are some good arguments here that may change my approach a bit. I may wind up agreeing with you.
Properties used in WPF data binding must be public, so there are places in the framework that force that choice.
My argument in the general case is that effectively internal can be inferred (e.g. by tooling), but only a developer can denote what is part of the public surface of the class. So using internal for all members discards information.
I prefer “internal” when I don’t want the compiler to bother me with a docstring I don’t want to write 😉
Which hopefully means your internal methods are just that., internal.
There’s a special place reserved in hell for people who create undocumented public APIs :p
There’s a big distance between “API which will be documented and maintained forevermore” and “API which no outside code would ever have a use for”. Between them is “API which outside programmers shouldn’t use without having received from the author documentation sufficient to determine that it will meet their needs for as long as it will be required.” Although it might be possible to manage such things using “InternalsVisibleTo”, that can create some versioning headaches. If a new version of module X will need to access some internals of Y (and the author of Y approves), and if Y has no other reason to change, upgrading X shouldn’t require upgrading Y.
What’s needed is a way of distinguishing between things which the public is welcome to use and rely upon, versus things which aren’t locked down but will have no guarantee of stability absent some outside promise of such.
I regularly deal with a large codebase that’s almost comment-free. I want an IDE that electrocutes coders who say that. I want no IDEs that don’t.
Plus one that detects class-level comments of the form “This implements the so-and-so class.”
You skipped the “protected internal” access modifier. And sadly its cousin, “private protected”, will never be.
Agree with your logic that every member used external to the class should be public unless there’s a strong reason not to.
What would be “private protected”? It doesn’t even make sense to me…
C# allows “protected internal” which means “the less restrictive combination of protected and internal”. That is, methods start as “private”, and “protected” widens their accessibility to subclasses, and “internal” widens their accessibility to the assembly, so “protected internal” widens accessibility to code in subclasses OR code in the assembly.
There has for a long time been a feature request to allow the more restrictive combination, that is, widening accessibility to only subclasses that are in the current assembly.
This kind of accessibility is not very interesting because it is basically a “my bozo coworkers keep on calling this method but I intend it only to be called from a subclass”. Well, if your bozo coworkers keep doing that, there’s probably a reason, and that reason is probably an unfulfilled need in the assembly that you should fix in a way that makes your coworkers happy, rather than slapping a more restrictive kind of accessibility on the thing and then waiting for them to complain. But still, people ask for it a lot.
There was a proposal for the feature to be added to C# 6, which then raises the question “what should the syntax be?” and one proposal was “private protected”, which is not great, but was better than anything else anyone came up with.
The feature was cut from further consideration on May 7th. As has always happened in the last decade of considering this feature, it was determined to be too much trouble for too little gain.
If a public class is only supposed to be inherited *within the assembly where it is declared*, it should be possible for that class to have members which should only be used by subclasses *on themselves*, of types which are only available within the assembly. Internal access is semantically overly permissive, but it’s the only kind of access other than the restrictive “protected and internal” which would be permissible under .NET rules.
Personally, I would like to see better means by which classes and interfaces could restrict derivation or implementation to the assembly where the class or interface is declared. Abstract classes can limit direct inheritance to their own assembly by including an abstract member of an internal type, but that trick isn’t usable with concrete classes. Because of the way finalizers are implemented, I know of no way by which a concrete inheritable C# class can prevent outside code written in another language from declaring a derived class whose `Finalize` method stores `this` somewhere and returns without chaining to the base finalizer, and a constructor which immediately throws; such code could then effectively construct a derived object without giving the base class any say in the matter.
I hear that sort of feedback a lot, and I am genuinely confused by it. Not by the content, which is on the face of it reasonable, but by the sheer amount of people who want even more complicated ways to interact with class hierarchies. More complicated ways to control inheritance, more complicated ways to control access, and so on. Why is there such a huge, decades-long focus on tweaking the small points of what is actually a pretty poor method for code reuse? Particularly since these small points are almost invariably “I want the compiler to prevent my coworkers from violating some design principle of the program”. OK, I understand that preventing coworkers from making mistakes is goodness, but if that’s what you want then why focus on inheritance?! That is not where most coworkers make most of their mistakes! I see real user-affecting bugs in real large programs every single day and probably not one tenth of one percent of them would have been prevented by having a better mechanism for restricting inheritance, but a huge fraction of them would be mitigated by, say, having a way to express workflow conditions in the language.
All the time programmers write code that has workflow restrictions. This socket must be closed the same number of times it is opened. This lock must be taken after than lock. The return type of this method must be checked for nullity every time it is called. And so on. The “contract” that is expressed by the public interface of a type is incredibly weak; C# gives you that a reference to an integer will not be passed when a reference to a string is needed, which is about the weakest kind of contract you can imagine. Violations of some sort of workflow contracts are a cause of a huge fraction of the bugs I see.
I am genuinely confused why people have been asking me, personally, for a decade to add even more complicated ways to restrict inheritance which would solve almost no real problems, but it is extremely rare to hear a request for a feature that would actually prevent the kinds of bugs that developers actually cause.
A concrete example of a situation where I can see usefulness to restricting inheritance would be the scenario where a collection wishes to perform the equivalent of `List(T).AddRange(IEnumerable(T))`, and the received collection is likely to be some form of array-backed collection. Enumerating through every item of the received collection will be much slower than using Array.Copy to do the bulk of the work copying from one collection directly to the other, but the source could only use Array.Copy if the destination exposed its array, and likewise the destination could only use it if the source exposed its array.
If the Runtime included some interfaces like `IReadableArray(out T)` which included a `CopyTo(int sourceIndex, Array dest, int destIndex, int count)` and `ISafeReadableArray(T) AsSafeReadableArray()`, with `ISafeReadableArray(out T) inheriting from IReadableArray but *only* being implementable by the Runtime, then it would be possible for code receiving a collection to pass a private array to `AsReadableArray().CopyTo()` and know that it was invoking a Runtime method which would not do anything improper with the passed-in array. It might be possible to use Runtime-defined structure or class instead of an interface, but there would be a number of very distinct implementations (including array types) some of which wouldn’t really fit an inheritance model.
The same “bozo coworkers” argument applies just as strongly to the ‘private’ modifier.
Well, if the language lacked *any* way to distinguish implementation details from public interface, then your coworkers aren’t bozos when they don’t read your mind correctly.
I think there’s a difference between “Internal isn’t restrictive enough; it’s crucial to the class to make sure this isn’t fiddled with.” and “Internal isn’t restrictive enough; I want to make sure this is only fiddled with by subclasses”. In the case of private, you’re setting an absolute “this represents an invariant of the base class, any outside modification is unhelpful and unnecessary” rule. The internal/protected only modifier is saying that it’s really allowed to be fiddled with, but only by people who know what they are doing… And as Eric has said elsewhere, if there are logical restrictions to how an implementation detail should be interacted with, you need more than an access modifier anyway.
Great post. I’ve always agreed with your position to prefer public accessibility within internal types, but I wonder now if there are other subjective situations similar to this involving accessibility.
Pingback: The Morning Brew - Chris Alcock » The Morning Brew #1695
I am clearly on the ‘internal’ camp, this is how it gets compiled.
>Another way to say that would be: suppose we marked the member as internal, and then decided to make the class public instead of internal. Would we have to change the member from internal to public in order for the class to function? If the answer is “yes” then it should have been public in the first place.
If the class move from internal to public, then it is time to review the whole class to see what should be public.
> public in C# means “accessible to anyone who can see the class”;
I always found this assumption misleading, because then public has two meanings and this makes things more complicated.
“accessible to anyone who can see the class” for members and nested types
“accessible to any others assemblies” for non-nested types
Personally I am in favor of strong encapsulation, where visibility is escalated only when required. And this is why with the static analysis tool I am developing there are these two default rules:
Methods that could have a lower visibility
http://www.ndepend.com/default-rules/webframe.html?Q_Methods_that_could_have_a_lower_visibility.html
Types that could have a lower visibility
http://www.ndepend.com/default-rules/webframe.html?Q_Types_that_could_have_a_lower_visibility.html
How does `public` represent two rules? Every type or member is contained within *something*, and public types, like other public members, are only visible to things that can see the thing in which they are contained. Even a type which isn’t contained within an outer-level type will still be contained in an assembly, and will only be visible to code *that can see that assembly*.
If ‘public’ would have simply mean “visible from others assemblies” clearly this would be easier to grasp than “visible from everywhere my container is visible” isn’t it?
But maybe it is like colors, we can hardly discuss which one we prefer?
If public meant “visible by anyone who can see the assembly”, what specifier would you use for something that’s nested in a non-public container, which should be visible to anyone who can see the container? Conversely, would there be any case where it would make sense for something to be visible to anyone that *couldn’t* see the container wherein it resided?
To be sure, I think it might be helpful to split ‘public’ into two forms, but I would distinguish between items which are guaranteed to be visible to outside assemblies, and continue to work the same way, in all future versions of the class, versus things which may need to be exposed to outside assemblies for technical reasons but might arbitrarily change without public notice (if the author promises to notify certain individuals if the features change, those people might safely use them even if the world at large cannot).
>what specifier would you use for something that’s nested in a non-public container, which should be visible to anyone who can see the container?
The same specifier as the container one. Of course this require maintenance in case of change, but this makes things clearer and encapsulation more obvious at code review time.
>I think it might be helpful to split ‘public’ into two forms
Btw, in the tool I am working on we added a IsPubliclyVisible vs. IsPublic to distinguish both needs:
http://www.ndepend.com/api/webframe.html?NDepend.API~NDepend.CodeModel.IMember_properties.html
Are you implying that if a container is private or protected, that things declared private within a nested type should be visible to anything that can see the container? What then would you use for things that shouldn’t be visible to things that can see the container?
As for distinguishing “IsPublic” from “IsPubliclyVisible”, that’s not quite what I’m after. Right now, within a publicly-visible container, everything is either locked down or has a sign that says “public welcome”. What I’d like would be a third alternative–things that aren’t locked down, but have a sign that says “use at own risk”. To use an analogy, I would like my gardener to be able to use the rain barrel next to my shed. If I put a lock on it, I’d have to somehow arrange for the gardener to get a key. If I don’t particularly care if anyone else uses the rain barrel, it would be easier to leave it unlocked. What’s important is that my failure to lock the barrel should not be regarded as an invitation for people to drink from it.
>If public meant “visible by anyone who can see the assembly”, what specifier would you use for something that’s nested in a non-public container, which should be visible to anyone who can see the container?
The same rule applies. Public if it must be seen by other assemblies (like a protected nested type, that can be seen by derived type in other assemblies) else Internal. And eventually ‘protected internal’ depending on the situation.
And then there’s also the ‘InternalsVisibleTo’ attribute class, which allows you to specify other assemblies that can access the internal stuff all the same. It’s used in frameworks that consist of a couple of assemblies that heavily depend on each other, but want to provide a lean interface to the outside world.
Pingback: Dew Drop – September 16, 2014 (#1856) | Morning Dew
My issue with using public methods inside internal classes is that doesn’t “read” well – It translates to something like like: “Others can’t see me…. but they can see something inside me.” Which doesn’t make a lot of sense.
I’d argue it’s better to have no public methods inside internal classes – just internal ones. If the class ever becomes public, the wise thing would then be to review each method and determine whether it should become public or remain internal.
Wouldn’t the wise thing be to use the appropriate accessibility in the first place? I mean, I’ve even used internal methods in internal classes even though I prefer public accessibility simply because internal was more appropriate in that situation. But you’re stating that public accessibility is never appropriate within an internal class?
I think we all agree that a well designed system exhibits one important characteristic: To change something you only have to adapt code in very few locations.
So far I haven’t seen anybody argue why this basic rule should be violated here and what advantages that really gives us.
I agree. Which leads me to wonder what I actually do in internal classes I write. Maybe I’m changing my mind away from my default position of “if I could restrict the permission further and it still compiles, then I probably should.”
> First of all, let’s establish that there is no technical difference between the two. public in C# means “accessible to anyone who can see the class”; making a public member of an internal class does not make the member more accessible than making it internal would.
What does this mean if A) the object is returned (with a static type of object, or an interface, or a superclass) to code outside the class, and then the caller uses reflection or dynamic or data binding to call the method (bonus: the method has the same name as an explicitly implemented function in an interface that the object implements)?
Suppose also that the caller does not have ReflectionPermission .
Your question can be answered by science. Try it and report your results!
Dynamic does not allow the call at all.
With ReflectionPermission denied (I think, I’m not sure if I did it right), neither method can be accessed.
Reflection (o.GetType().InvokeMember) without any permission restrictions will fail to find the internal-declared function unless BindingFlags.NonPublic is included, which is a visible difference.
Reblogged this on Jarek Przygódzki. Blog programisty.
PragDave said something here that I think would apply: “When faced with two or more alternatives that deliver roughly the same value, take the path that makes future change easier.”
Wouldn’t it be better and clear if the members of a type derive their access modifiers directly from the type itself? And they will use explicit access modifiers only if they have to narrow their access.
For example, if we have an internal class C, all of the members of C without access modifier will be internal (public modifier for the members of C will cause compile-time error).
If one decides to make C public, all of its members without access modifiers will get automatically public (and vice versa).
In this way, one does not have the “strange case” public member in internal type … the logic for every member is straightforward – “I cannot have more access than my container (only less eventually)”.
Thanks Eric for this discussion! If you ask me the source of the problem is Microsoft and the people that build this accessor system. It feels incomplete as scoping and security of assemblies and types is not hierarchical nor granular as it could have been.
So you know, the so-called top-level types inside or outside the namespace of each application are always “internal” by default. If you just say “class ABC” its internal in scope by default and excludes access from all other assemblies. Its also “public” by default to all assembly types by default, as well. That means by default all .NET C# projects have a form of sealed assembly where all types are accessible internally only.
What then should the scope be of nested types and members? By default most are private. I think enums and interfaces are public by default. But what does it matter if your project top level types are internal in most cases by default anyway?
That’s why we should all develop our nested types and members as if their top-level parents are internal and sealed off from other assemblies and allow them to be public or private relative to their own assembly, always, and not worry about outside assembly access. In that sense they should function as public, private, and protected members relative to the top-level types only, avoiding “internal” accessors, and ignoring any future exposure as you will control that in the top-level types anyway should you expose them later that way. By avoiding nested internals and placing internal in your top-level types only, you now have a nice fluid contained project that’s not worrying itself with future accessibility. The .NET system isnt designed to allow much more than that anyway. If it had granular controls for accessibility and scoping it would have avoided the scenarios you describe. But they didnt. In an insecure world that’s a bad idea, if you ask me.
So using the top-level classes and types should always be explicitly internal unless you have decided to open it up to the world and go public. In both cases however, the nesting of subtypes and members must be independent of whats in the top-level types as their functioning should be relevant to the sub types and members they interact with, not the worries for future assembly access.
Pingback: Why use a public method in an internal class? – Knowleage Exchange
Pingback: c++ - pública o interna a los métodos de una clase interna
A bit late to the conversation.
The whole debate stems from the fact, that people don’t specify more precisely what they mean when they say “public” or “internal”. Some debaters go with the “effectively internal” argument, some with the “intentionally internal”. Intention and end effect are, clearly, different things. An open door in a closed building is still open, even though you can’t enter it from the street.
I believe “public” and “internal” are simply different modifiers each with their own isolated and non-overlapping set of use cases, and saying that one is interchangeable with another in any situation is simply invalidating the very definition of those modifiers. Saying that just because two certain combinations (public in internal and internal in internal) achieve “effectively” the same level of access doesn’t make those combinations identical. Not even close. Any argument in favor of them being identical by semantics is simply incorrect, regardless of the end effect. Are sentences “I’m cold” and “I’m not warm” identical? No. However similarly meaning they are in a given situation.
What would really help when structuring one’s argumentation is perhaps prepending “locally” and “globally” to the access modifier. C# as a language only operates with the “local” term, when actually prepending the modifier to an entity. If something has the, say, “public” access modifier it’s strictly and unconditionally “public” within its container (be that assembly or type). There is no “yes, but”. C# language spec to the rescue. However, whether or not that same something is also “public” “globally” would require to evaluate the entire chain of containment, before you are allowed to say if something “locally public” is also “globally public”.
Let’s have a 2×2 matrix:
* public members of public types – classic
* public members of internal types – typical situation of hiding the entire type from the outside world.
* internal members of public types – when types are used from outside but there is a bit of hacky-wacky access play within the assembly itself
* internal members of internal types – I fail to see any real world example for this, but the effect (not the semantics) is seemingly identical to the public members in internal types. Perhaps this one is when you may one day want to make the class public, but those sneaky internal members will remain internal for your assembly-level trickery.
There seems to be a dilemma where there is none. You can use both access modifiers, but in zero cases interchangeably. You just need to decide first on what you mean. Figure out the semantics. And if you didn’t decide what you mean, don’t go writing code yet.
Cheers
I’m totally stealing the “an open door inside a closed building is still an open door” analogy!
Pingback: Why use a public method in an internal class? – w3toppers.com