4

Summary: if I create an object in a constructor initialiser, how do I keep a reference to it so I can reference it later?

Details:

I have a class (LibBase, below) that requires a StreamWriter as its construction parameter. I don't have the source code for LibBase - its in a third-party library.

public class LibBase
{
    public LibBase(System.IO.StreamWriter wtr) { ... }
}

I have derived MyClass from LibBase and in the MyClass constructor I want to pass an instance of MyWriter (derived form StreamWriter) to the base class. I do this as follows.

public class MyWriter : System.IO.StreamWriter
{
    public MyWriter(int n) { ... }
    // Contains unmanaged resources
}

public class MyClass : LibBase
{
    public MyClass(int n)
    : LibBase(new MyWriter(n))
    { }
}

The problem is that MyWriter requires disposing, so MyClass should dispose it (and implement IDisposable to do this) but MyClass doesn't have a reference to the created MyWriter instance, so I can't dispose it. The syntax for the constructor initialiser doesn't seem to permit my to keep a reference.

My solution is to re-code MyClass as follows:

public class MyClass : LibBase, IDisposable
{
    public MyClass(Encoding enc)
    : this(new MyWriter(enc))
    { }

    private MyClass(MyWriter wtr)
    : LibBase(wtr)
    { this.wtr = wtr; }  // store reference

    private MyWriter wtr;

    // (implement IDisposable using wtr member variable
}

The private constructor stores a reference to the MyWriter instance so I can dispose it later.

My questions:

  1. What am I missing here? I feel like I'm fighting the language. Does C# provide a better way to do this?
  2. If the language doesn't directly support this, is there a better solution than my private constructor technique?
  3. Any comments on defects in my solution?
Andy Johnson
  • 7,938
  • 4
  • 33
  • 50

3 Answers3

5

I don't think you're missing anything here. Your solution looks okay to me if LibBase really doesn't let you get at the writer you passed in to the constructor.

I suspect that the reason there isn't more explicit support for this is that it doesn't crop up very often. If you find it cropping up very often in your designs, it's possible that you're overusing inheritance.

To channel Eric Lippert:

However, as I often point out, I don't have to provide a justification for not doing a feature. Features aren't cheap; they are extremely expensive and they must not only justify their own cost, they must justify the opportunity cost of not doing the hundred other features we could have done with that budget. We must justify the cost of features to our stakeholders, but we need not justify saving time and effort by not implementing features that don't meet our bar.

My guess this is something which doesn't meet the bar, even if it were deemed desirable in the first place. (There's ongoing cost beyond the cost to the C# team - there's the mental cost of each feature being absorbed by C# developers, and the ongoing cost that each new feature makes it potentially harder to add the next feature.)

Community
  • 1
  • 1
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Thanks for the response. Unfortunately LibBase doesn't provide access to the writer. This is the first time I've encountered this problem, but its nice to know I'm not missing something obvious. – Andy Johnson Oct 06 '11 at 11:54
2

Your solution seems ok... I don't think that you miss anything...

IF you want to change the implementation (for whatever reason):

  • You could implement that by not inheriting from LibBase but having an instance as a private member...
  • another option would be to implement the Factory pattern for MyClass thus having no public constructor and creating the StreamWriter instance factory-side etc.

BUT as I said there is nothing wrong with you solution (if it happens often you probably should rethink your design).

EDIT - as per comment:

What I mean by "creating StreamWriter factory-side" is: create a Factory for MyClass so that anyone needing an instance uses the Factory... therein you can create the StreamWriter instance in the Factory method and pass it in as param to MyClass... this way you could even implement some fancy things like "which MyClass instance is using a given StreamWriter instance ?" or some sort of a cache for MyClass / StreamWriter instances etc.

Yahia
  • 69,653
  • 9
  • 115
  • 144
  • Thans for your answer. See my comments on Jan Hudec's answer for my reasons for not using a private `LibBase` member variable. Could you elaborate on what you mean by "creating the StreamWriter instance factory-side"? Sounds interesting. – Andy Johnson Oct 06 '11 at 12:26
  • Thanks. I didn't realise you meant changing the constructor signature to pass in the writer. Using a factory would certainly work and is arguably more elegant. The existing code doesn't use factories, so it would be a divergence from style, but I may use that approach. – Andy Johnson Oct 06 '11 at 13:02
  • Accepted for suggestion to use a factory. I think I was so fixated on doing this with a language-level construct that is blinded me to the obvious code pattern to use. – Andy Johnson Oct 06 '11 at 14:01
1

I think that in this case you shouldn't be deriving LibBase but delegating to it. In which case you can obviously initialize the members in any order you want.

Anybody dealing with MyClass has to handle the disposal, but code written to use LibBase does not, so you can't simply throw instance of MyClass at code written to handle LibBase. In such cases inheritance is not really appropriate. Of course if LibBase doubles as an interface, you can't really help it in which case your workaround seems like the best thing you can do.

Jan Hudec
  • 73,652
  • 13
  • 125
  • 172
  • Thanks for your helpful answer. I didn't consider delegation in this case for two reasons. (1) `LibBase` has a lot of public methods which I would have to implement wrappers for in `MyClass`. (2) other classes in the third-party library deal with references to `LibBase` instances, so if I used delegation then I couldn't use a `MyClass` instance as a `LibBase` (I think this is what you mean by "if LibBase doubles as an interface"). But I agree, in other circumstances delegation would be better. It would be even better if `LibBase` was a real `interface` ... – Andy Johnson Oct 06 '11 at 12:23