Should accessible members of an internal class be internal too?
- by Jeff Mercado
I'm designing a set of APIs for some applications I'm working on. I want to keep the code style consistent in all the classes I write but I've found that there are a few inconsistencies that I'm introducing and I don't know what the best way to resolve them is.
My example here is specific to C# but this would apply to any language with similar mechanisms.
There are a few classes that I need for implementation purposes that I don't necessarily want to expose in the API so I make them internal whereever needed.
Generally what I would do is design the class as I normally would (e.g., make members public/protected/private where necessary) and change the visibility level of the class itself to internal. So I might have a few classes that look like this:
internal interface IMyItem
{
ItemSet AddTo(ItemSet set);
}
internal class _SmallItem : IMyItem
{
private readonly /* parameters */;
public _SmallItem(/* small item parameters */) { /* ... */ }
public ItemSet AddTo(ItemSet set) { /* ... */ }
}
internal abstract class _CompositeItem: IMyItem
{
private readonly /* parameters */;
public _CompositeItem(/* composite item parameters */) { /* ... */ }
public abstract object UsefulInformation { get; }
protected void HelperMethod(/* parameters */) { /* ... */ }
}
internal class _BigItem : _CompositeItem
{
private readonly /* parameters */;
public _BigItem(/* big item parameters */) { /* ... */ }
public override object UsefulInformation
{
get { /* ... */ }
}
public ItemSet AddTo(ItemSet set) { /* ... */ }
}
In another generated class (part of a parser/scanner), there is a structure that contains fields for all possible values it can represent. The class generated is internal too but I have control over the visibility of the members and decided to make them internal as well.
internal partial struct ValueType
{
internal string String;
internal ItemSet ItemSet;
internal IMyItem MyItem;
}
internal class TokenValue
{
internal static int EQ(ItemSetScanner scanner) { /* ... */ }
internal static int NAME(ItemSetScanner scanner, string value) { /* ... */ }
internal static int VALUE(ItemSetScanner scanner, string value) { /* ... */ }
//...
}
To me, this feels odd because the first set of classes, I didn't necessarily have to make some members public, they very well could have been made internal. internal members of an internal type can only be accessed internally anyway so why make them public? I just don't like the idea that the way I write my classes has to change drastically (i.e., change all uses of public to internal) just because the class is internal.
Any thoughts on what I should do here?
It makes sense to me that I might want to make some members of a class declared public, internal. But it's less clear to me when the class is declared internal.