1

Background

I have an object that represents a test procedure that I need to run on a product. This object may be used multiple times during run-time, depending on how many times the user elects to run the test. Here is where the design challenge starts. This TestProcedure object contains many members that maintain their values throughout the object's lifetime.

When the object is instantiated, its members need to have specific initial values. So, if the operator ends the test and decides to restart it, those members no longer have any meaning. With that being said... if I don't reset the members before running another test, the initial values will be incorrect and therefore invalidate the test.

Rather than resetting each and every member back to the value it should be, here is what I am doing instead...

private TestProcedure myProcedure = null;
private CancellationTokenSource myTokenSource = new CancellationTokenSource();
private CancellationToken myToken = CancellationToken.None;

// creates and executes a test procedure
public void Run()
{
    // get a new token source
    this.myTokenSource = new CancellationTokenSource();

    // get a new token
    this.myToken = myTokenSource.Token;

    // instantiate a new procedure with the token
    myProcedure = new TestProcedure(myTokenSource);

    // do the work inside the procedure
    myProcedure.DoWork();
}

So then, where I see a problem is that when the user goes to create a re-run the test... Run will get called, so in-turn I am just overwriting the old object with a new instance. I feel that this may not be the correct way of doing things. I feel that I am mis-managing the memory here.

Question

Isn't this one of those situations where we are supposed to use a using statement and implement IDisposable, or can I get away with just doing things the way I have them now?

  • Is your concern performance, memory usage, stale state, reset state, or ...? – Robert Harvey May 2 '16 at 14:34
  • @RobertHarvey Stale state, member values that don't reflect reality. – Snoop May 2 '16 at 14:43
  • @RobertHarvey Well then again, I don't think so... It's true that the members won't be what I need them to be, that is unless I reset the object to it's initial state. So, reset state? Definitely not performance, and I think that "memory usage" could be a concern, but only in a sense that I am overwriting the old object with the new instance. – Snoop May 2 '16 at 14:46
  • 1
    I'm confused, you have a myProcedure field and talk about overwriting something (presumably that field?), but then your Run() never uses this field, and I don't see any reason for the field to exist. – svick May 2 '16 at 14:58
  • 1
    @StevieV In that case, what is the purpose of that field? Why not just use a local variable? – svick May 2 '16 at 15:09
4

If TestProcedure contains exclusively managed objects (as opposed to the resources such as database connections, GDI+ objects, etc.), there is nothing you need to do. Just create a new instance of TestProcedure at the beginning of the test, and let Garbage Collector handle the removal of the object when the test finishes.

If TestProcedure uses objects which should be released as soon as possible, such as a connection to a database, then:

  • Either ensure that the resource is used within a using () { } statement within the TestProcedure itself in order for it to be disposed immediately after you end using it. For instance, instead of keeping a SqlConnection instance inside TestProcedure, provide within TestProcedure the methods which open a connection when called, and close/dispose it immediately before exiting.

  • Or make TestProcedure implement IDisposable if the previous approach requires too much code (which could happen if you use the same unmanaged resource a lot or if you use a lot of different unmanaged resources).

Talking about your concerns about the values that don't reflect reality, make sure the instances are local to the Run method, since you don't seem to use the variable anywhere else. In other words, instead of:

private TestProcedure myProcedure = null;

public void Run()
{
    ...
    myProcedure = new TestProcedure(myTokenSource);
    myProcedure.DoWork();
}

do:

public void Run()
{
    ...
    new TestProcedure(myTokenSource).DoWork();
}

Every call to Run will create a new instance of the TestProcedure class; if the method is called multiple times, the values from the old invocation won't “pollute” the context of a newer invocation. Every instance will be kept as soon as it's needed, and then removed by Garbage Collector.

  • Did you see what RH asked about in his comment? I should have included that as part of my question (and may need to do so) does this address that? – Snoop May 2 '16 at 14:53
  • No, it wouldn't invalidate your answer at all. I guess technically it would still address the concern of reset-state, but I guess... does that make it a suitable way of doing it? – Snoop May 2 '16 at 15:00
  • No. Nothing in parallel, only sequentially. I would essentially Run then Stop then re-instantiate... then call Run again. – Snoop May 2 '16 at 15:05
  • Does that help anything? – Snoop May 2 '16 at 15:07
  • Yeah, see the TestProcedure has to run for a long time, so when I call a Stop method that will change the value of the token allowing the TestProcedure to cancel whatever its doing. – Snoop May 2 '16 at 15:09
2

The main question over whether you should implement IDisposable is if you are using any unmanged resources.

From the IDisposable Documentation

The primary use of this interface is to release unmanaged resources. The garbage collector automatically releases the memory allocated to a managed object when that object is no longer used. However, it is not possible to predict when garbage collection will occur. Furthermore, the garbage collector has no knowledge of unmanaged resources such as window handles, or open files and streams.

Use the Dispose method of this interface to explicitly release unmanaged resources in conjunction with the garbage collector. The consumer of an object can call this method when the object is no longer needed.

If nothing you use is out side the .net framework (and call dispose / wrap in using statements when needed) then garbage collection will take care of cleaning up every thing and you don't need to worry.

I tend to implement IDisposible in the following circumstances as well as when dealing with unmanaged resources:

Whenever a class level resource implements IDisposable (IE: a class that accesses an Entity Framework context) that I want to dispose of correctly.

In libraries where most of the other classes have implemented it for the above 2 reasons and I want to maintain a consistent interface. This is purely stylistic and is not needed but when reading code if I always wrap code from that library in a using statement it makes things easier to read.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.