Proposal: fields reinitialization

[astef]

Problem

"Clearing" or "resetting" is a widely met operation. We can find it both in a low-level classes ( List.Clear, Stopwatch.Reset, DataSet.Reset), and in a high-level APIs: (Legend.Clear, Console.Clear, Control.ControlCollection.Clear)

In a messy enterprise code, "resetting" of a complex object state almost always means a lot of bugs, because after adding this operation, state natural evolution (adding fields, changing field types, etc.) must be "mirrored" in the resetting procedure. What's even worse, after a few iterations some inaccuracies during reset can be recognized as a valid side-effects, making object state analysis much more complicated.

Solution

It would be very useful, if the run-time could propose a conventional way of resetting. The language can expose it in a very laconic way:

class MyContainer
{
    private readonly int _capacity;

    [Reinitializing]
    public MyContainer(int capacity)
    {
        _capacity = capacity;
    }

    public void Clear()
    {
        this(_capacity);
    }
}

We're marking constructor with language-supported ReinitializingAttribute, which allows it to be called at run-time (with reflection) and at compile-time (like in MyContainer.Clear method).

[canton7]

From your example, it's not clear why you can't have a helper method which is called from both your ctor and your Clear method?

[spydacarnage]

I assume that's because the field is marked as readonly, which then can't be modified outside of the constructor. However, bring able to mutate a readonly field after it's initial construction is against the purpose of the readonly flag in the first place, so I don't like the idea.

…

On Sat, 4 Jan 2020, 11:56 Antony Male, ***@***.***> wrote: From your example, it's not clear why you can't have a helper method which is called from both your ctor and your Clear method? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <https://github.com/dotnet/csharplang/issues/3066?email_source=notifications&email_token=ADIEDQJT23MZNISVWPETGFLQ4B2ODA5CNFSM4KCVTWRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEICWOOQ#issuecomment-570779450>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADIEDQNNVCDBXQINS2SCIALQ4B2ODANCNFSM4KCVTWRA> .

[canton7]

The readonly field is only assigned to its current value in the example, so that assignment could simply be removed. That's why I asked.

Any Clear method written today would only be able to assign to non-readonly fields, and there's nothing in the OP about allowing readonly fields to be assigned after construction

[astef]

@canton7 Good point, example is not perfect.

The problem is that initialization is only a part of the job. Another part - is clearing the existing state to default values. For example, let's add this method for MyContainer:

public void Add(object item)
{
    // ...
    _count++;
}

_count field here is definitely something we want to be "cleared" to a default value. Therefore, the helper method becomes a typical buggy "resetting" procedure, which has to clean everything up. And real evil is that with this helper method we're giving the possibility to intentionally not to do a full initialization and silently "loose" it's real meaning after adding a new field.

[astef]

@spydacarnage regarding your point about readonly fields.

readonly fields are guaranteed not to be changed after initialization. This guarantee will remain.

1. Let's consider private readonly fields first. Then the proposed feature is almost equivalent to this pattern:

class MyContainerHost
{
    private MyContainer _myContainer = new MyContainer();

    public MyContainerHost()
    {
        _myContainer.ReinitializationNeeded += Reinitialize;
    }

    private void Reinitialize()
    {
        _myContainer = new MyContainer();
    }
}

Where MyContainer has the private readonly field, which is being re-initialized. It is a bad practice, and the proposed feature makes it easier to fall into it, but the original intention - loose the entire state just to change readonly field, - is too artificial, especially with an option of simply removing the readonly keyword.

2. Non-private readonly fields are much more dangerous, because class user/inheritor can rely on the value not to change after object construction. That's one of the reasons why ReinitializingAttribute or other explicit sign of a potentially breaking change is needed.

But this is not kind of feature which should be recommended to be added everywhere or almost everywhere, like nullability attributes. And in a rare situations, where an old code can be advised to be changed to reinitialization, the deep analysis of class usages is a must:

public Guid UniqueId { get; } = Guid.NewGuid();

The above property, if being saved by an external user, should obviously block you from moving to reinitialization.

[spydacarnage]

Unless I'm missing something, I still disagree. Your code, as written, explicitly allows the mutation of a readonly field, even though that mutation occurs by re-running the constructor.

…

On Sat, 4 Jan 2020, 12:56 Alexander Stefurishin, ***@***.***> wrote: @spydacarnage <https://github.com/spydacarnage> regarding your point about readonly fields. readonly fields are guaranteed not to be changed after initialization. This guarantee will remain. 1. Let's consider private readonly fields first. Then the proposed feature is almost equivalent to this pattern: class MyContainerHost { private MyContainer _myContainer = new MyContainer(); public MyContainerHost() { _myContainer.ReinitializationNeeded += Reinitialize; } private void Reinitialize() { _myContainer = new MyContainer(); } } Where MyContainer has the private readonly field, which is being re-initialized. It is a bad practice, and the proposed feature makes it easier to fall into it, but the original intention - loose the entire state just to change readonly field, - is too artificial, especially with an option of simply removing the readonly keyword. 1. Non-private readonly fields are much more dangerous, because class user/inheritor can rely on the value not to change after object construction. That's one of the reasons why ReinitializingAttribute or other *explicit* sign of a potentially breaking change is needed. But this is not kind of feature which should be recommended to be added everywhere or *almost* everywhere, like nullability attributes. And in a rare situations, where an old code can be advised to be changed to reinitialization, the deep analysis of class usages is a must: public Guid UniqueId { get; } = Guid.NewGuid(); The above property, if being saved by an external user, should obviously block you from moving to reinitialization. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/dotnet/csharplang/issues/3066?email_source=notifications&email_token=ADIEDQOSNSCBIOD74IVPMADQ4CBOTA5CNFSM4KCVTWRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEICXNOY#issuecomment-570783419>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADIEDQI4ICAXLFR4G6KU3VTQ4CBOTANCNFSM4KCVTWRA> .

[ronnygunawan]

How about adopting C++'s placement new operator?

// Create a new instance
MyContainer mc = new MyContainer(10);

// Create a new instance and replace object referenced by mc with the new instance
// Finalizer will be called on the old instance after successful creation of the new instance
mc = new (mc) MyContainer(5);

[theunrepentantgeek]

"Clearing" or "resetting" is a widely met operation.

I disagree. I've very seldom seen a need for this outside of dedicated reusable data structures such as List<T>, Queue<T> and so on, where the need for the operation is a part of the usual contract.

It doesn't appear to me to be anywhere common enough to require dedicated support from either the language or the base class library.

In a messy enterprise code, "resetting" of a complex object state almost always means a lot of bugs, because after adding this operation, state natural evolution (adding fields, changing field types, etc.) must be "mirrored" in the resetting procedure.

When you do have an object that genuinely needs a formal reset/clear operation, it's not difficult to write a unit test that checks all the fields are properly reset, providing a safety net for any future maintenance work. Similar unit tests are also useful for ensuring consistency across equality tests, hash code generation, unsaved change detection, and so on.