Is it a bad programming practice to have "Public" members inside an "Internal" class?

Question

Wouldn't it be more specific and appropriate if I only keep "protected", "internal" and "private" members (field, method, property, event) in a class which is declared as "internal"?

I have seen this practice (having "public" members in an "internal" class) in various code so just wanted to know is it a bad practice or does it has some benefit or advantage.

[Only concerned about C#] Thanks for your interest.

Westbrooks

Answer 1

Not necessarily. If you want to implicitly implement an interface, then public members are more than acceptable.

Generally though, if the class is internal, public members don't make much sense. You won't get hurt, since you won't be able to expose the class in a strongly typed way outside of the module it is defined in, but if you aren't implicitly implementing an interface, there isn't much advantage.

Bagging

+1; it really bothers me that we can't have an implicit internal interface implementation for internal interfaces though. – Lomond

You can implement the interface's members as private methods. ebersys.blogspot.com/2009/04/… – Stern

@Julien Lebosquain: What do you mean by that? It doesn't make much sense to have an implementation that is internal for an interface that is more visible. It's kind of like saying a class can be public and internal at the same time. – Bagging

@Justin: Actually, you can't. You are referring to explicit implementation of interfaces, and the members have the same visibility as the interface does. If you cast the implementation to the interface, you have access to the member. If it was truly private, you would never have access to that member. – Bagging

Thanks for the update. I've never used the 'explicit implementation' in any code. I had just come across this post and remember reading post I referenced above. Thanks for shedding some light on that :) – Stern

Answer 2

It's reasonable to assume that these public members are still part of the public interface of the class, even if the class itself has internal scope. When I see internal on a class member, this says to me 'somewhat dodgy backdoor access expressing tight coupling, whereas public still implies proper defensive programming responsibilities in my mind. It's purely a conceptual distinction.

Mort

+1 IMHO, internal members of public classes are half-encapsulated. I avoid them unless absolutely necessary (which I find to be rare). – Azotobacter

When I've found myself using internal members, it usually means that I'm mixing up representing data with acting on that data in a complex, stateful way. I recently did a major refactoring to remove this kind of stateful processing (hidden as 'internal') that never should have been stored as part of the classes being processed. – Mort

Answer 3

The internal specification on the class restricts the scope of the public declaration on the members, so all of your public members are really internal. That said, public is a lot less typing than internal, so my normal idiom is to declare the class as internal and the exposed methods as public. I only mark a member as internal if the class is public and I want to restrict some members to the same assembly.

Tabriz

'a lot less typing' as an argument to choose between language keywords; obviously this answer is not sponsored by the Speed Typists Association of America – Decolonize

Answer 4

That's what I do. Can't help myself, when my brain thinks "this member should be accessible", my fingers uncontrollably start hammering p u b l i c. No such problem when I declare the class though.

One big advantage: a refactoring that make the internal class public (or the other way around preferrably) requires changing just one word. And Microsoft did this too, mostly.

Riggins

Answer 5

The only functional reason for this I am aware of is for implicitly implementing interfaces. All interface methods must be tagged with public in order to match up with the interface. This is true even if the type or interface is non-public.

Barring this limited situation, I really dislike the practice. Doing this makes it a bit harder to grep for the public surface area of your application. Instead you have to do a more advanced search.

Additionally it bothers me, in probably a bit of an irrational way, that members are marked as public when really they aren't.

Alexandrite

Makes you wonder why the compiler isn't optimized to downgrade public to internal, or warn, when an interface method isn't being used. No compiler warning or info provided (Fx 3.5) seems really weird to me and makes me wonder if there's not another reason this is being allowed that we haven't hit upon yet. – Decolonize

@jdk, hopefully Eric will be by later to comment on why but my guess is that the compiler doesn't warn because it's not actually incorrect. The CLI does allow for this type of accessibility inconsistency so unless there is a specific business justification for disallowing it the C# team decided to spend their resources elsewhere. – Alexandrite

We need a summon Eric button like Batman has, but I think it would be abused. – Decolonize

Answer 6

I try to imagine the possibility of the class access modifier changing when I set the method modifier. If a method needs to be internal even if the class is public I'll say so. If not then the method is written as public.

Bax

Answer 7

I would say yes it is bad programming practice.

It's not just about controlling access. It's also about making code more modular.

If a consumer of the data of an object wants some of that object's data, it calls the getter method. If, later, the data to be returned should be different, the programmer need only change the code in one place instead of all the places where that data was read (if the member was public). This is similar to why we should write functions/methods and call them instead of just copying and pasting code about the place.

Explained with an example here: http://www.programmingincpp.com/private-versus-public-data-members-in-a-class.html

Perceive