42

I'm a pretty new C# and .NET developer. I recently created an MMC snapin using C# and was gratified by how easy it was to do, especially after hearing a lot of horror stories by some other developers in my organisation about how hard it is to do in C++.

I pretty much went through the whole project at some point and made every instance of the "public" keyword to "internal", except as required by the runtime in order to run the snapin. What is your feeling on this, should you generally make classes and methods public or internal?

Pang
  • 8,605
  • 144
  • 77
  • 113
1800 INFORMATION
  • 119,313
  • 29
  • 152
  • 234

14 Answers14

39

I believe in blackboxes where possible. As a programmer, I want a well defined blackbox which I can easily drop into my systems, and have it work. I give it values, call the appropriate methods, and then get my results back out of it.

To that end, give me only the functionality that the class needs to expose to work.

Consider an elevator. To get it to go to a floor, I push a button. That's the public interface to the black box which activates all the functions needed to get the elevator to the desired floor.

Stephen Wrighton
  • 33,316
  • 6
  • 63
  • 84
13

What you did is exactly what you should do; give your classes the most minimal visibility you can. Heck, if you want to really go whole hog, you can make everything internal (at most) and use the InternalsVisibleTo attribute, so that you can separate your functionality but still not expose it to the unknown outside world.

The only reason to make things public is that you're packaging your project in several DLLs and/or EXEs and (for whatever reason) you don't care to use InternalsVisibleTo, or you're creating a library for use by third-parties. But even in a library for use by third-parties, you should try to reduce the "surface area" wherever possible; the more classes you have available, the more confusing your library will be.

In C#, one good way to ensure you're using the minimum visibility possible is to leave off the visibility modifiers until you need them. Everything in C# defaults to the least visibility possible: internal for classes, and private for class members and inner classes.

Pang
  • 8,605
  • 144
  • 77
  • 113
Ryan Lundy
  • 187,365
  • 35
  • 174
  • 206
7

I think you should err on the side of internal classes and members. You can always increase an item's visibility but decreasing it can cause problems. This is especially true if you are building a framework for others.

You do need to be careful though not to hide useful functionality from your users. There are many useful methods in the .NET BCL that cannot be used without resorting to reflection. However, by hiding these methods, the surface area of what has to be tested and maintained is reduced.

Pang
  • 8,605
  • 144
  • 77
  • 113
Brownie
  • 7,570
  • 5
  • 25
  • 38
4

I prefer to avoid marking classes as public unless I explicitly want my customer to consume them, and I am prepared to support them.

Instead of marking a class as internal, I leave the accessibility blank. This way, public stands out to the eye as something notable. (The exception, of course, is nested classes, which have to be marked if they are to be visible even in the same assembly.)

Jay Bazuzi
  • 41,333
  • 12
  • 104
  • 161
2

Most classes should be internal, but most non-private members should be public.

The question you should ask about a member is "if the class were made public would I want to member the member to be exposed?". The answer is usually "yes (so public)" because classes without any accessible members are not much use! internal members do have a role; they are 'back-door access' meant only for close relatives that live in the same assembly.

Even if your class remains internal, it is nice to see which are front-door members and which are back-door. And if you ever change it to public you are not going to have to go back and think about which are which.

James
  • 4,549
  • 1
  • 30
  • 39
1

You should tend toward exposing as little as possible to other classes, and think carefully about what you do expose and why.

Pang
  • 8,605
  • 144
  • 77
  • 113
ColinD
  • 103,631
  • 27
  • 195
  • 199
1

Is there any reason you need to use Internal instead of Private? You do realise that Internal has assembly level scope. In other words Internal classes/members are accessible to all classes in a multi-class assembly.

As some other answers have said, in general go for the highest level of encapsulation as possible (ie private) unless you actually need internal/protected/public.

Ash
  • 57,515
  • 31
  • 146
  • 168
1

I found a problem using internal classes as much as possible. You cannot have methods, properties, fields, etc of that type (or parameter type or return type) more visible than internal. This leads to have constructors that are internal, as well as properties. This shouldn't be a problem, but as a matter of fact, when using Visual Studio and the xaml designer, there are problems. False positive errors are detected by the designer due to the fact that the methods are not public, user control properties seems not visible to the designer. I don't know if others have already fallen on such issues...

Mike
  • 63
  • 2
  • 6
  • 1
    This is wrong. You can have public methods and properties on an internal class, indeed doing that is recommended by many people. – kjbartel Aug 26 '14 at 08:13
1

You should try to make them only as visible as possible, but as stated by Mike above, this causes problems with UserControls and using the VS Designer with those controls on forms or other UserControls.

So as a general rule, keep all classes and UserControls that you aren't adding using the Designer only as visible as they need to be. But if you are creating a UserControl that you want to use in the Designer (even if that's within the same assembly), you will need to make sure that the UserControl class, its default constructor, and any properties and events, are made public for the designer to work with it.

I had a problem recently where the designer would keep removing the this.myControl = new MyControl() line from the InitializeComponent() method because the UserControl MyControl was marked as internal along with its constructor.

It's really a bug I think because even if they are marked as internal they still show up in the Toolbox to add in the Designer, either Microsoft needs to only show public controls with public constructors, or they need to make it work with internal controls as well.

Pang
  • 8,605
  • 144
  • 77
  • 113
Aaron Murgatroyd
  • 1,644
  • 17
  • 18
0

It depends on how much control you have over code that consumes it. In my Java development, I make all my stuff public final by default because getters are annoying. However, I also have the luxury of being able to change anything in my codebase whenever I want. In the past, when I've had to release code to consumers, I've always used private variables and getters.

Heath Borders
  • 27,732
  • 16
  • 126
  • 230
0

I like to expose things as little as possible. Private, protected, internal, public: give classes, variables, properties, and functions the least amount of visibility they need for everything to still work.

I'll bump something's visibility up that chain toward public only when there's a good reason to.

Matt Blaine
  • 1,926
  • 14
  • 21
0

by default class is created as internal in c#: internal means: Access is limited to the current assembly.

see http://msdn.microsoft.com/en-us/library/0b0thckt.aspx

Good Article the defaults scope is internal: http://www.c-sharpcorner.com/UploadFile/84c85b/default-scope-of-a-C-Sharp-class/

0

I completely disagree with the answers so far. I feel that internal is a horrid idea, preventing another assembly from inheriting your types, or even using your internal types should the need for a workaround come about.

Today, I had to use reflection in order to get to the internals of a System.Data.DataTable (I have to build a datatable lightning fast, without all of its checks), and I had to use reflection, since not a single type was available to me; they were all marked as internal.

Peder Rice
  • 1,754
  • 3
  • 26
  • 49
  • 1
    `internal` exists so that API vendors (such as MS) can write classes without worrying about random strangers using their code. Whenever an API becomes public, the vendor can no longer easily change the signatures. When `internal`, if MS chooses to, they can change their code without fear of breaking other people's code. That is, unless they are using reflection to "cheat". – Kirk Woll Apr 15 '11 at 22:58
  • I contend that you can do the same with private and protected members so that at least when (and I say "when", not "if") someone dives in, they can use strong typing instead of reflection. – Peder Rice Apr 16 '11 at 16:34
  • 1
    @Pedar, how does using `private` make it at all easier to use strong typing rather than reflection? `private` is even **less** accessible than `internal`. And if you use `protected` it is now part of the **contract** of the API and MS will be prohibited from changing it. I think you ignored that point I made. – Kirk Woll Apr 16 '11 at 16:46
  • @KirkWoll: I think the issue is not so much strangers using their code, but rather strangers coming to rely upon details that may in fact be subject to change without notice. – supercat Sep 12 '13 at 16:11
-1

Do not choose a "default". Pick what best fits the visibility needs for that particular class. When you choose a new class in Visual Studio, the template is created as:

class Class1
{
}

Which is private (since no scope is specified). It is up to you to specify scope for the class (or leave as private). There should be a reason to expose the class.

Pang
  • 8,605
  • 144
  • 77
  • 113
Ryan Farley
  • 11,169
  • 4
  • 43
  • 43
  • In what circumstances would you choose internal, public or private? What particular needs might influence you to choose one? – 1800 INFORMATION Sep 20 '08 at 03:30
  • internal and private are identical for non-nested classes, as far as I know: both are visible only to the entire assembly. (or does it make a difference when using InternalsVisibleTo attribute?) – Tobi Sep 20 '08 at 10:20
  • 7
    "Top-level types, which are not nested in other types, can only have internal or public accessibility. The default accessibility for these types is internal." (http://msdn.microsoft.com/en-us/library/ba0a1yw2.aspx) – Auron Nov 03 '08 at 16:57