Proposal: "part private" access modifier within partial classes

[mkupf]

Proposal

Add a part private access modifier to restrict access to a member of a partial class to its class part. This would enable us to do some lightweight "micro encapsulation" within large classes:

partial class LargeClass
{
    part private object _lock = new object();

    part private int _c;

    part private void _check()
    {
        ...
    }

    private void Increment()
    {
        lock(_lock)
        {
            _check();
            _c++;
        }
    }
}

partial class LargeClass
{
    // can not access _lock, _c and _check() from here

    // can access Increment() from here
}

Explanation

It is not uncommon to have large classes containing a lot of members. Especially most private members are only to be accessed within a small region and not from anywhere else in the class.

The Thing with Regions

This kind of leads to the highly divisive discussion of the #region statement and in particular to the question whether it is necessary to have a mechanism of lightweight encapsulation within classes. But since #region is there I think the philosophy of the language is clear on that and should be continued consequently.

In my opinion the problem with this discussion is, that its about making things lightweight and the advantage of things being lightweight can not be demonstrated easily in simple examples, because that is a thing which starts to take effect in real world scenarios.

The Traditional Way

Of course in traditional oop you would introduce classes to achieve my proposed mechanism of encapsulation, but that is a heavy weight solution including optimization unfriendly object pointers and I think people love C# because it offers simple solutions for everyday problems, like properties which also could be implemented traditionally by bare get and set methods.

Introducing classes every time you want a little encapsulation also results in over modeling and lots of not problem related code to wire up those instances. It also results in lots of classes, written for special cases living in tight one to one relationships.

At first sight one can cope with this, but in real world scenarios this is where reading code would start to get complicated because naming those classes either results in really long names or results in unclear names. Thats why in such cases one quits encapsulation because its not worth the effort. But if made possible easy, such kind of micro encapsulation could improve code readability more often.

Another Example

Consider a situation, where you have a central repository class answering various questions. For instance you can add cars and streets and you can ask for the amount of red and blue cars added and you can also ask for the total length of streets added.

This could look something like this:

class A
{
    #region Cars

    private List<Car> _cars = new List<Car>();

    private int _red = 0;

    private int _blue = 0;

    public void Add(Car c)
    {
        _cars.Add(c);
        if (c.Color == Colors.Red) _red++;
        else if (c.Color == Colors.Blue) _blue++;
    }

    public int Red => _red;

    public int Blue => _blue;

    #endregion

    #region Streets

    private List<Street> _streets = new List<Street>();

    private double _length = 0;

    public void Add(Street s)
    {
        _streets.Add(s);
        _length += s.Length;
    }

    public double Length => _length;

    #endregion
}

Actually here you don't want people outside the cars region to mess up those red and blue counters or to add cars directly to _cars which is essential to guarantee this data structure to work.

Do you really want to introduce a RedAndBlueCarsCountingListOfCars just to enforce this?

And do you really want to introduce a TotalLengthOfStreetsMeasuringListOfStreets that is only used one single time?

And if you have that, do you really want to wire up all those?

When It Comes to View Models

Especially when it comes to view models things start to get even worth, because we have to take care of passing change notifications and so on.

Here another common case is a section of properties when implementing INotifyPropertyChanged where you actually would want to force anybody in a large view model to use the notifying setters instead of their backing storages:

class B : INotifyPropertyChanged
{
    ...

    private int _number;

    public int Number
    {
        get { return _number; }
        set
        {
            _number = value;
            OnPropertyChanged(...);
        }
    }

    private string _text;

    public string Text
    {
        get { return _text; }
        set
        {
            _text = value;
            OnPropertyChanged(...);
        }
    }
}

Making those "part private" could simply do the trick here.

Other Considerations

Note part private could also be a great deal for safely separating generated code from hand written code.

Because all of this is tightly related to regions an alternate proposal would be to introduce some kind of region private modifier, to restrict access of a member to its surrounding region. In terms of usability I think this would even be much nicer and this would be even more lightweight and would also make it possible to nest scopes to arbitrary level, but since #region statements are preprocessor trivia this unfortunately would mix up different layers of the language.

What do you think?

As you may have noticed I named those part private members with a leading underscore making their use feel more fragile, as they are kind of low level. I used this as a convention for myself for years to not access underscore named members from outside their declaring region. I even wrote a roslyn analyzer to enforce this.

Related Discussions

• https://whathecode.wordpress.com/2010/12/07/function-hell/
• https://stackoverflow.com/questions/3265637/blocking-access-to-private-member-variables-force-use-of-public-properties

[theunrepentantgeek]

Overly large classes lead to code that's unmaintainable. There's a reason why the single responsibility principle is talked about so much. I'm not convinced we should be adding language features solely to encourage larger classes (keep in mind that partial classes were introduced to allow easier integration with code-generation, allowing generators to completely rewrite files without loss of handwritten code).

Introducing classes every time you want a little encapsulation also results in over modeling

Sometimes it does, yes. Often it doesn't. Check out, for example, the archetypes of colour modelling where separate aspects of a system are deliberately split out into different types.

Your repository example is interesting because there's a straightforward solution that delivers the encapsulation you want without needing any extra features or any awkward class names.

class Repository
{
    public CarRepository Cars { get; }
	public StreetRepository Streets { get; }
}

class CarRepository 
{
    private List<Car> _cars = new List<Car>();
	
	private FilteredList<Car> _redCars = new FilteredList<Car>( c => c.Colors == Colors.Red );
	private FilteredList<Car> _blueCars = new FilteredList<Car>( c => c.Colors == Colors.Blue );

    public void Add(Car c)
    {
        _cars.Add(c);
		_redCars.Add(c);
		_blueCars.Add(c);
    }

    public int Red => _redCars.Count;
    public int Blue => _blueCars.Count;
}

class StreetRepository
{
    private List<Street> _streets = new List<Street>();

    private double _length = 0;

    public void Add(Street s)
    {
        _streets.Add(s);
        _length += s.Length;
    }

    public double Length => _length;
}

class FilteredList<T> : IList
{
	private readonly Predicate<T> _filter;
	
	public FilteredList(Predicate<T> filter) { ... }
}

I didn't introduce RedAndBlueCarsCountingListOfCars because the name is too specific to the situation. Instead, I introduced FilteredList<T> as a way to generically solve the problem. This way, I can easily introduce _greenCars at a later date if needed. (Actually, as soon as I hit three kinds of lists, I'd get rid of the hard coded member variables and instead use something that scaled for additional rules as required.)

[CyrusNajmabadi]

A nifty way to handle htis is when nested structs. So you can do the following:

class MyComplexClass {
     private struct NestedFunctionalityGroupA { state and members };
     private struct NestedFunctionalityGroupB { state and members };

    void SomeComplexFunc()
    {
        return new NestedFunctionalityGroupA(this).Func();
    }
}

This allows you to effectively namespace your state and operations into their own logical groupings, without requiring splitting into sibling types, or taking perf hits from object-allocations. Because the nested type is a struct, it state effectively is just space taken up on the local sstack in SomeComplexFunc. All function calls are also direct.

This addresses your concern about:

Of course in traditional oop you would introduce classes to achieve my proposed mechanism of encapsulation, but that is a heavy weight solution including optimization unfriendly object pointers

While also giving you the encapsulation that you want (i.e. no peeking into privates in these structs).

--

Roslyn uses this pattern itself in many places when some complex task needs to happen and:

1. we don't want the allocation hit of creating a new object for each time the operation runs.
2. we don't want to make mutable objects that get 're-initialized' each time.

[mkupf]

Hello Bevan,

thank you for your detailed answer! I get the point as I wrote under "The Traditional Way". And I understand that philosophy.

But lets abstract your traditionally true

Overly large classes lead to code that's unmaintainable.

to: "Overly large capsules lead to code that's unmaintainable."

This difference is important because the idea of this proposal was to introduce a lightweight encapsulation layer below classes, allowing us to write larger maintainable classes.

Suppose you were an alien looking at a village with one room houses placed in alphabetical order like in our beloved solution explorer and you would want to understand what is going on. Then it would be much easier if every family had a multiroom house instead of being spread all over the place.

Of course this is arguable and it depends on the question what a class exactly is.

But I think it would be great if we could think of a class in a slightly more conceptual way. In the traditional solution you posted, you introduced classes in order to technically structure the code. But in my opinion there should be a difference between the conceptual structure and the technical structure. Technical you want access restrictions etc., but conceptual, when you think about the problem, I think one would think of the one repository entity and not of the parts of its implementation, because that is of course implementation specific.

Of course we want reutilization of implementation parts (mostly hidden from the user), but you changed the whole API in order to make it straigtforward. In a special case that may not make big difference. But in general it breaks the separation between conceptual structure and implementation.

What if there would be another implementation storing everything in one place so that there are no two separate parts of implementation anymore?

I thought of three more realistic examples:

1. If you implement some kind of binary tree structure and you want to implement IEnumerable. Then all this is only one aspect of the whole data structure and you would have to wire up things when everything should be passed into other instances.

2. As another thing consider implementing System.ComponentModel.ICollectionView, an interface with lots of members.

3. I think most of those WPF classes for instance follow that pattern to only introduce seen classes for conceptual things. I mean most of them have lots of members. I don't want to blow this up with too many long samples, but for instance WPF classes have lots of virtual methods one can override in derived classes. This is a lightweight interface but would lead to much boilerplate if the super classes API would be split into disjunct parts.

I would be interested in what you would say why there is the #region statement in C#? :)

Because the #region statements purpose is to introduce an encapsulation layer below classes I think.

As you might have guessed I'm just so sad that it can't be used to create real access restricted capsules ;-)

I know there are many people against the #region statement. So if it's not part of the philosophy behind the language anymore then of course I understand that C# classes are to be small. But for me the introduction of local functions for instance is a sign in the opposite direction. That is the same thing on another level meaning the question wheather we need a mechanism to encapsulate below methods. And that seems to be answered with yes.

[CyrusNajmabadi]

That is the same thing on another level meaning the question wheather we need a mechanism to encapsulate below methods. And that seems to be answered with yes.

As i pointed out, you can do that today with inner classes (or inner structs if you want to avoid the allocation cost).

[mkupf]

@CyrusNajmabadi Good point! I didn't know that! I tried out nested classes for that, but nested structs are much more nifty and it represents the wanted structure in a good way :)

[mkupf]

(sorry, just saw your comments too late)

[theunrepentantgeek]

@mkupf asked:

I would be interested in what you would say why there is the #region statement in C#? :)

I'd suggest there's pretty strong evidence that #region was introduced into C# as a way to hide boilerplate or generated code that the developer should be ignoring.

Consider the original visual designer for Windows.Forms - this would serialize the UX into a #region. While the serialization looked like C# (after all, the compiler handled it just fine), many a developer soon learned that it wasn't really C# - even trivial changes to the code would completely break the visual designer. The wrapping #region was a useful signpost don't mess with this (or, more colourfully, here be dragons).

There's also the behaviour of every version Visual Studio - have you noticed that each #region shows up collapsed by default? That's no accident - it's trying to help you focus on the code that matters, the code outside the region.

FWIW, I worked on a team that standardized on the use of regions to group parts of each class - after a couple of years of finding everything important hidden by default, and having to click multiple times to unhide things a hundred times every day, we reversed course and declared #region as something that should be rarely used, and only for hiding unimportant code that should be ignored.

[mkupf]

@theunrepentantgeek thanks for your response.

Take a look at the official implementation of System.Windows.Controls.Calendar. A 1700 lines class with the structure in it of what I was thinking of:

For instance in Line 41 there is the member private bool _isShiftPressed; and it is only used in two methods. In private void ProcessSelection(...) in line 1596 and in private void ProcessShiftKeyUp() in lines 1652 and 1655. I think one would improve readability if one would group this little selection system together. I see one could do that with nested structs, but I think this still would not be leightweight enough to really improve readability.

One could also discuss this with a class like System.Windows.Controls.DataGrid, having 8700 lines. Just let me quote some of its private variable declarations. You can see all those little subsystems each consisting of about one to three variables:

        private DataGridColumnCollection _columns;                          // Stores the columns
        private ContainerTracking<DataGridRow> _rowTrackingRoot;            // Root of a linked list of active row containers
        private DataGridColumnHeadersPresenter _columnHeadersPresenter;     // headers presenter for sending down notifications
        private DataGridCell _currentCellContainer;                         // Reference to the cell container corresponding to CurrentCell (use CurrentCellContainer property instead)
        private DataGridCell _pendingCurrentCellContainer;                  // Reference to the cell container that will become the current cell
        private SelectedCellsCollection _selectedCells;                     // Stores the selected cells
        private List<ItemInfo> _pendingInfos;                               // Selected items whose index is not yet known
        private Nullable<DataGridCellInfo> _selectionAnchor;                // For doing extended selection
        private bool _isDraggingSelection;                                  // Whether a drag select is being performed
        private bool _isRowDragging;                                        // Whether a drag select is being done on rows
        private Panel _internalItemsHost;                                   // Workaround for not having access to ItemsHost
        private ScrollViewer _internalScrollHost;                           // Scroll viewer of the datagrid
        private ScrollContentPresenter _internalScrollContentPresenter = null; // Scroll Content Presenter of DataGrid's ScrollViewer
        private DispatcherTimer _autoScrollTimer;                           // Timer to tick auto-scroll
        private bool _hasAutoScrolled;                                      // Whether an auto-scroll has occurred since starting the tick
        private VirtualizedCellInfoCollection _pendingSelectedCells;        // Cells that were selected that haven't gone through SelectedCellsChanged
        private VirtualizedCellInfoCollection _pendingUnselectedCells;      // Cells that were unselected that haven't gone through SelectedCellsChanged
        private bool _measureNeverInvoked = true;                           // Flag used to track if measure was invoked atleast once. Particularly used for AutoGeneration.
        private bool _updatingSelectedCells = false;                        // Whether to defer notifying that SelectedCells changed.
        private Visibility _placeholderVisibility = Visibility.Collapsed;   // The visibility used for the Placeholder container.  It may not exist at all times, so it's stored on the DG.
        private Point _dragPoint;                                           // Used to detect if a drag actually occurred
        private List<int> _groupingSortDescriptionIndices = null;           // List to hold the indices of SortDescriptions added for the sake of GroupDescriptions.
        private bool _ignoreSortDescriptionsChange = false;                 // Flag used to neglect the SortDescriptionCollection changes in the CollectionChanged listener.
        private bool _sortingStarted = false;                               // Flag used to track if Sorting ever started or not.
        private ObservableCollection<ValidationRule> _rowValidationRules;   // Stores the row ValidationRule's
        private BindingGroup _defaultBindingGroup;                          // Cached copy of the BindingGroup created for row validation...so we dont stomp on user set ItemBindingGroup
        private ItemInfo _editingRowInfo = null;                            // Current editing row info
        private bool _hasCellValidationError;                               // An unsuccessful cell commit occurred
        private bool _hasRowValidationError;                                // An unsuccessful row commit occurred
        private IEnumerable _cachedItemsSource = null;                      // Reference to the ItemsSource instance, used to clear SortDescriptions on ItemsSource change
        private DataGridItemAttachedStorage _itemAttachedStorage = new DataGridItemAttachedStorage(); // Holds data about the items that's need for row virtualization
        private bool _viewportWidthChangeNotificationPending = false;       // Flag to indicate if a viewport width change reuest is already queued.
        private double _originalViewportWidth = 0.0;                        // Holds the original viewport width between multiple viewport width changes
        private double _finalViewportWidth = 0.0;                           // Holds the final viewport width between multiple viewport width changes
        private Dictionary<DataGridColumn, CellAutomationValueHolder> _editingCellAutomationValueHolders
                    = new Dictionary<DataGridColumn, CellAutomationValueHolder>(); // Holds the content of edited cells. Required for raising Automation events.
        private DataGridCell _focusedCell = null;                           // Holds the cell which has logical focus.
        private bool _newItemMarginComputationPending = false;           // Flag to indicate if row margin computation request is pending

Ok, I think my thought is clear. And I understand many people don't like it. So anyway it would not be good to introduce a feature disliked by many people because that would lead to inconsistent coding styles. So I guess this time I have to assimilate. Thanks for your time! :)

[Thaina]

I like it

It don't need to be very large class. Just partial class that split in 3 files would make me want to scope variable that would be used only in one file. Because most of the time one part of partial class would do only related logic with only some related field. Other part just should not aware of unused field

In my opinion. Every place should not need to aware of thing they don't need to. That's why I support many issue relate to scoping such as static keyword in method

[MkazemAkhgary]

@Thaina @mkupf

Because most of the time one part of partial class would do only related logic with only some related field. Other part just should not aware of unused field

then please go back and read first comment more carefully.

---

for code generators I think they could apply [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] attribute so those fields does not show up in intellisense.

[AustinBryan]

If your class is getting so big you need to hide part of it from itself, I think that is very much a sign that should just become it's own class and that it's violating SRP.