3

I am trying to create a general method for disposing an object that implements IDisposable, called DisposeObject()

To make sure I am disposing an object pointed by original reference, I am trying to pass an object by reference.

But I am getting a compilation error that says

The 'ref' argument type doesn't match parameter type

In the below (simplified) code, both _Baz and _Bar implement IDisposable.

alt text

So the questions are,

  1. Why am I getting this error?
  2. Is there a way to get around it?

[UPDATE] From provided answers so far, as long as I do not set an IDisposable argument to null, I can simply pass an object by value without using ref. I am now having another trouble whether to set disposable objects to null or not within DisposeObject method.

Here is the full source for completeness:

public class Foo : IDisposable
{
    private Bar _Bar;
    private Baz _Baz;
    private bool _IsDisposed;

    ~Foo() { Dispose(false); }

    public void Dispose(bool disposing)
    {
        if (!_IsDisposed)
        {
            if (disposing)
            {
                DisposeObject(ref _Baz);
                DisposeObject(ref _Bar);
            }
        }

        _IsDisposed = true;
    }

    private void DisposeObject(ref IDisposable obj)
    {
        try
        {
            if (obj == null) 
                return;
            obj.Dispose();
            obj = null;
        } catch (ObjectDisposedException) { /* Already Disposed... */ }
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
}

public class Bar : IDisposable
{
    public void Dispose() {}
}

public class Baz : IDisposable
{
    public void Dispose() {}
}

[RESULT]
I removed the code that sets argument to null (obj = null;) within DisposeObject So the final code became.

    public void Dispose(bool disposing)
    {
        if (!_IsDisposed)
        {
            if (disposing)
            {
                DisposeObject(_Baz);
                DisposeObject(_Bar);
            }
        }

        _IsDisposed = true;
    }

    private void DisposeObject(IDisposable obj)
    {
        try
        {
            if (obj == null) 
                return;
            obj.Dispose();
        } catch (ObjectDisposedException) { /* Already Disposed... */ }
    }
Community
  • 1
  • 1
dance2die
  • 35,807
  • 39
  • 131
  • 194

5 Answers5

8

Here's an option for your example (can't verify it against a compiler right now, but you'll get the idea):

private void DisposeObject<T>(ref T obj) where T : class, IDisposable
{
    // same implementation
}

To call it, use

DisposeObject<Baz>(ref _Baz);
DisposeObject<Bar>(ref _Bar);

As pointed in the other comments, the compiler error you get has its own purpose (preventing you to assign some other type of IDisposable inside your method, leading to an inconsistent state).

Dan C.
  • 3,643
  • 2
  • 21
  • 19
  • That's nifty. It compiles but since I decided not to set an argument to null, I have already selected an answer. I think I will be able to use above code for future uses. Thanks Dan C. – dance2die Apr 27 '09 at 16:46
  • Very clever, and most of the time you can rely on type reference (e.g. `DisposeObject(ref _Baz)`, `DisposeObject(ref _Bar)`) – Ohad Schneider Jul 03 '11 at 13:29
  • 1
    Note that if try to do `obj = null;` (which was the original reason OP used `ref`), will get a compiler error on the attempt to assign null. To fix this, change constraint to `where T : class, IDisposable`. – ToolmakerSteve Oct 08 '21 at 05:01
  • @ToolmakerSteve, thanks for pointing that out, I added it to the answer for any future readers – Dan C. Mar 11 '22 at 11:45
4

Try this:

IDisposable d = (IDisposable)_Baz;
DisposeObject(ref d);

Edit: As Adam points out, your code doesn't require this to be ref. Objects are always passed as references.

Jon B
  • 51,025
  • 31
  • 133
  • 161
  • That doesn't work actually. You get an error that "the ref or out argument must be an assignable variable". – Noldorin Apr 27 '09 at 15:54
  • Edited per Noldorin's comment. – Jon B Apr 27 '09 at 15:56
  • Yeah, that should do the trick now. Though I still don't think there's any reason for the OP to use ref parameters here. – Noldorin Apr 27 '09 at 15:58
  • No, there isn't. It just adds needless complexity. – Adam Robinson Apr 27 '09 at 15:59
  • 1
    @dance2die, no, VARIABLES are passed by value. However, this variable holds a REFERENCE type, so the reference itself (not the content of the object) is what's being passed by value. This is similar to your other thread in that regard. The only functional difference that the ref keyword gives you is the ability to assign a value to that variable and have it reflected in the calling code. You aren't doing that, so the ref keyword is unneeded. – Adam Robinson Apr 27 '09 at 16:02
  • @dance2die - that link is broken, but that isn't right. Objects are by ref and primitives are by value. – Jon B Apr 27 '09 at 16:03
  • 1
    @Jon B: Objects references are passed by value. Objects are not passed by [ref] by default: you cannot assign a new object reference to an actual parameter and see that value after the call in the calling function. – Brian Ensink Apr 27 '09 at 16:03
  • @Jon B: SO is recognizing " as part of URL. let me repost it. – dance2die Apr 27 '09 at 16:04
  • @Jon B: As far as I have read on http://www.yoda.arachsys.com/csharp/parameters.html (by Jon Skeet), objects are passed by "value" by default – dance2die Apr 27 '09 at 16:04
  • 3
    @danc2die: Brian is absolutely correct. The *reference* is passed by value by default. The object itself isn't passed at all. I don't believe my article claims that they are... I certainly hope it doesn't say that. – Jon Skeet Apr 27 '09 at 16:06
  • Yes, the refernce is passed by value. It's a little mind bending when you think of it that way. But it's a reference - that's the important part. You're not passing the object, but a reference to it. Hence, you don't use the ref keyword. – Jon B Apr 27 '09 at 16:06
  • Clarified the by ref statement in my answer. – Jon B Apr 27 '09 at 16:08
  • 3
    @Jon B: Unless you want to change the value of the caller's variable, in which case passing the argument by reference makes perfect sense. Claiming that "objects are by ref" confuses the two issues IMO. It's much simpler and more accurate to just understand that the value of a reference type variable is a reference, not an object. At that point, it's all nice and consistent. The "objects are by ref" explanation gets really muddy when the parameter is by ref *as well*. – Jon Skeet Apr 27 '09 at 16:09
  • @Jon: While your code does compile, it violates the spirit of dance's original intent (which, admittedly, doesn't seem possible) by only assigning your temporary variable not null, not the original instance variable. – Adam Robinson Apr 27 '09 at 16:17
4

There is no need for you to pass by reference, as you're passing a reference type. You should remove the ref keyword from your method definition. Do this and you shouldn't have any issues, though I'm not sure how this is more effective or clearer than simply calling Dispose() (other than the fact that you don't have to cast it for explicit implementations and this does a null check for you).

Edit

Dance, while I hope the discussion that's surrounded this topic has been helpful to you, your original intent doesn't seem to be something that's doable. In order to pass something as ref, you can't pass a variable that has a type other than what the ref parameter expects (in other words, you can't pass a variable declared as a class or other interface that implements IDisposable if the ref parameter is IDisposable). Because ref parameters allow assignments to propagate back to the caller, you would open up the possibility of allowing incompatible types being stored in your variable.

Your best bet here is to assign null yourself if that's what you want. If you want to encapsulate the null check and ignoring exceptions in to the function that's fine, but ref will not work for you in this scenario no matter how you slice it, unfortunately.

Adam Robinson
  • 182,639
  • 35
  • 285
  • 343
  • I wanted to delegate "NULL" and "ObjectDisposedException" to a method instead of having to write "try..catch" for each object. – dance2die Apr 27 '09 at 15:56
  • I agree with you, Adam. I would guess he's doing it this way so that the object reference is nullified so that if DisposeObject is called again with the same reference, it'll exit early. – Michael Haren Apr 27 '09 at 15:57
  • 2
    Per Microsoft guidelines, calling Dispose() should never cause an ObjectDisposedException (or, more generally, repeated calls to Dispose() should never generate an error). Even so, there's no need for the ref keyword. It's only adding complexity and greater accessibility requirements. You should remove it. – Adam Robinson Apr 27 '09 at 15:58
  • @Michael: I might agree with you if the object referenced were assigned to null ;) – Adam Robinson Apr 27 '09 at 15:58
  • You might be right on the fact that, if I am just calling "Dispose()" on a method without setting _Bar or _Baz to null within "DisposeObject", I don't think I would need "ref". – dance2die Apr 27 '09 at 15:58
  • @dance: According to your code, that is what you're doing. Ref parameters are, in general, a "code smell". There's no real need for you to use them in this scenario. – Adam Robinson Apr 27 '09 at 16:00
  • Marked as Answer: I have decided not to set argument to null within "DisposeObject". And therefore there was no need for "ref". – dance2die Apr 27 '09 at 16:17
  • *There is no need for you to pass by reference, as you're passing a reference type.* There are plenty of legitimate reasons to pass a reference type by reference, when what you're interested in is the reference and not the object its pointing at. If you've ever used double pointers in C++ you'll understand. – MattDavey Jun 02 '11 at 13:49
  • @Matt: Apologies; my statement wasn't intended to communicate that it's never required to pass a reference type by reference, but that in the OP's particular situation it was (and is) not required. – Adam Robinson Jun 02 '11 at 17:13
2

This approach smells funny but I'll ignore that for now.

To fix your problem, you need to cast the objects you are passing with "(IDisposable)"

I concede to the will of the compiler, and Jon Skeet. You need an actual object for this:

IDisposable _BazD = (IDisposable)_Baz;
DisposeObject(ref _BazD);

I'd also add a null check in your DisposeObject() in addition to the try/catch. The "obj==null" will be a quick and easy check when compared to expensive exception catching should this get hit multiple times for the same object. Hmm...was that there a minute ago? Nevermind.

Michael Haren
  • 105,752
  • 40
  • 168
  • 205
  • Why is it that I would have to *explictly* cast the object as "IDisposable"? Can the compiler infer that I am actually passing an object that implements IDisposable? – dance2die Apr 27 '09 at 15:55
  • Sure it could, but it doesn't. – Michael Haren Apr 27 '09 at 15:56
  • @Michael: The ref keyword is what's limiting that. Were the ref keyword not there it would do exactly that. – Adam Robinson Apr 27 '09 at 16:03
  • 1
    That doesn't work. You'll get error CS1510: A ref or out argument must be an assignable variable. The problem is that the method could assign *any* IDisposable value to the variable, but the caller's variable is specifically a Bar or Baz. – Jon Skeet Apr 27 '09 at 16:04
  • @Michael Haren: As Jon Skeet has pointed out, your answer generates the following error. "ref" argument is not classified as a variable – dance2die Apr 27 '09 at 16:08
  • Also the fact that (correct me if I'm wrong), casting creates (or at least can create) another reference value (ie, another spot in memory to hold a reference, which may or may not hold the same reference as the original object). For example, if you created an explicit conversion on your object to IDisposable that returned a reference to a new/different object (which would likely be nonsense but is possible), then you would be passing this new reference as your ref parameter, not the original variable. – Adam Robinson Apr 27 '09 at 16:08
  • thanks for the feedback. i've updated my code ref to match @Jon B. I'd delete my answer as it's now worthless, but the comments are nice so I'll leave it. – Michael Haren Apr 27 '09 at 16:11
0

Thank you Dan C. I don't have enough rep yet to add comments, so I have to add this as an answer. However, full credit to Dan C for this solution.

This is working code:

public override void Dispose()
{
    base.Dispose();

    DisposeOf<UserTableAdapter>(ref userAdapter);
    DisposeOf<ProductsTableAdapter>(ref productsAdapter);

    if (connection != null)
    {
        if (connection.State == ConnectionState.Open)
        {
            connection.Close();
        }
        DisposeOf<SqlConnection>(ref connection);
    }
}

private void DisposeOf<T>(ref T objectToDispose) where T : IDisposable
{
    if (objectToDispose != null)
    {
        objectToDispose.Dispose();
        objectToDispose = default(T);
    }
}
Thorin
  • 626
  • 4
  • 11