-1

i have this base (abstract) class:

public abstract class StreamWriterBuilderService : IStreamWriterService
{
    private Stream fs;
    private StreamWriter stream;

    public void WriteLine(
        string stringToWrite )
    {
        if ( stream == null )
        {
            throw new InvalidOperationException( "Tried to call WriteLine on an uninitialised StreamWriterBuilderService" );
        }

        stream.WriteLine( stringToWrite );
        Flush();
    }

    public void Flush()
    {
        if ( stream == null )
        {
            throw new InvalidOperationException( "Tried to call Flush on an uninitialised StreamWriterBuilderService" );
        }

        stream.Flush();
    }

    public void Initialise(
        string filePath )
    {
        Contract.Requires<ArgumentNullException>( filePath != null );

        fs = File.Open( filePath, FileMode.Append, FileAccess.Write, FileShare.ReadWrite );
        stream = new StreamWriter( fs );
        WriteLine("Initialised");
        Initialised = true;
    }

    public bool Initialised { get; private set; }

    #region Dispose Implementation

    ~StreamWriterBuilderService()
    {
        Dispose( false );
        Debug.WriteLine( false, "This StreamWriterBuilderService object was not disposed of" );
    }

    private bool _isDisposed;

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

    protected virtual void Dispose(
        bool disposing )
    {
        if ( _isDisposed )
            return;

        if ( disposing && Initialised )
        {
            stream.Dispose();
            fs.Dispose();
            stream = null;
            fs = null;
        }

        _isDisposed = true;
    }

    #endregion Dispose Implementation
}

I am unit testing this with:

    protected readonly string TargetFilePath = Path.GetTempFileName();
    protected const string Header = "Initialised";

    protected override void SetContext()
    {
        SUT = Substitute.For<StreamWriterBuilderService>();
    }

and...

    private string fileContents;
    private const string TestString = @"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789!£$%^&*()+~@?><|¬";

    protected override void Because()
    {
        base.Because();
        SUT.Initialise( TargetFilePath );
        SUT.WriteLine( TestString );
        SUT.Flush();
        SUT.Dispose();
        fileContents = File.ReadAllText( TargetFilePath );
    }

The trouble I am having is that this:

SUT.Dispose();

does not actually call my Dispose method in my abstract class because it is declared as a virtual method. If I remove the virtual method then the code is played out. However, I need to keep the virtual method there as I need to override elsewhere in my solution...

Andrew Simpson
  • 6,883
  • 11
  • 79
  • 179
  • 1
    Given the standard pattern does not specify `virtual` on the parameterless Dispose(), can you talk us through why you added it? https://learn.microsoft.com/pt-br/dotnet/standard/design-guidelines/dispose-pattern – mjwills Aug 17 '17 at 10:13
  • @mjwills Thanks for posting. Well i have not added it. It was already there. (work). But, if the abstract class needs to dispose of things and if any child class may also need to dispose of things then surely I would need to use virtual so I can call its base and the childs dispose? Always like to learn.. what would you advise as an alternative? – Andrew Simpson Aug 17 '17 at 10:15
  • @mjwills Thanks. Sorry i was multi-typing :) did not notice the link durrhh.. thanks will read now +1 – Andrew Simpson Aug 17 '17 at 10:18
  • @mjwills quire right. Not sure why that virtual keyword was added. Removed it (thanks). However, The Dispose(True) does not call the virtual method in this scenario. I may have to use a concrete class off this base instead :( – Andrew Simpson Aug 17 '17 at 10:22
  • @mjwills But how? the base is used with the nsubstitute.for and not as a base – Andrew Simpson Aug 17 '17 at 10:24

2 Answers2

2

You should use:

var SUT = Substitute.ForPartsOf<StreamWriterBuilderService>();

This gives you fine control over the specific things you want to substitute. Thus, by default, your Dispose methods won't be substituted.

Alternatively, my recommendation (for this specific scenario) would be to build your own StreamWriterBuilderServiceForTesting class (that inherits from StreamWriterBuilderService). Because NSubstitute would not be involved, you could define its behaviour however you like.

mjwills
  • 23,389
  • 6
  • 40
  • 63
  • 1
    Not sure what happened to my previous comment, I had said 'thanks for your answer. I had used the latter. But had not realised the former would do what I want to'. With reflection I would have deleted this question as it seems a simple answer. I was just looking in the wrong places. thanks – Andrew Simpson Aug 17 '17 at 11:06
1

When you substitute a class like this, the virtual methods are replaced by the substitute. You probably want a partial substitute replacing only Dispose(bool).

Documentation about partial substitutes

and the second Paragraph here:

For starters, NSubstitute can only work with virtual members of the class, so any non-virtual code in the class will actually execute!

That is, virtual methods will not execute.

I'd create a derived type for testing purposes just to remove the abstract, and then test with that.

Haukinger
  • 10,420
  • 2
  • 15
  • 28
  • Can be done with `ForPartsOf` substituting `Dispose(bool)`, if you don't want to substitute anything, just use the original class (or derive trivially from it if it's `abstract`) – Haukinger Aug 17 '17 at 10:08
  • Haukinger - Thanks. I am readings docs now. If you have an answer you could post then.. :) – Andrew Simpson Aug 17 '17 at 10:10