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

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 i开发者_高级运维s 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.

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.

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.

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.

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.

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.

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.

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