I have a class MyClass that requires instances of A, B and C to be created for initialization. All three classes inherit IDisposable. I do not have access to the source code of A, B and C, but their constructors may throw an exception for certain parameter combinations. I know that throwing an exception in a constructor is bad practice because it may leave the class in an inconsistent state in memory (invariant violation). So instead of using a direct constructor, I use a factory method MyClass.Create() that initializes A, B and C inside. The Create function throws an exception if any of A, B and C throws an exception. I know that if I fail to create B, I should at least free A and raise the exception. Also I know that if I fail to create class C, I should at least free previously created A and B, and then raise exception in Create function.

Please tell me, is there a good pattern or code example, how I can clean up previously initialized parts in case when I fail to build class MyClass? What if there are more than three parts? I am thinking about using Stack<IDisposable> stack to clean up parts.

class A: IDisposable
{
  private readonly int p_value;

  public A(int value)
  {
    ArgumentOutOfRangeException.ThrowIfNegative(value);
    p_value = value;
  }

  public void Print()
  {
    Console.WriteLine($"Value is: {p_value}");
  }

  public void Dispose()
  {
    Console.WriteLine("A disposed");
  }
}


class B : IDisposable
{
  private readonly int p_value;

  public B(int value)
  {
    ArgumentOutOfRangeException.ThrowIfLessThan(value, 10);
    p_value = value;
  }

  public void Print()
  {
    Console.WriteLine($"Value is: {p_value}");
  }

  public void Dispose()
  {
    Console.WriteLine("B disposed");
  }
}


class C : IDisposable
{
  private readonly int p_value;

  public C(int value)
  {
    ArgumentOutOfRangeException.ThrowIfLessThan(value, 20);
    p_value = value;
  }

  public void Print()
  {
    Console.WriteLine($"Value is: {p_value}");
  }

  public void Dispose()
  {
    Console.WriteLine("C disposed");
  }
}

class MyClass: IDisposable
{
  private readonly A a;
  private readonly B b;
  private readonly C c;
  private bool disposedValue;

  protected MyClass(A _a, B _b, C _c) { a = _a; b = _b; c = _c;}

  public MyClass Create(int _value)
  {
    // Maybe throw error
    A a = new A(_value);

    // Maybe throw error
    // Need free A
    B b = new B(_value);

    // Maybe throw error
    // Need free A
    // Need free B
    C c = new C(_value);

    return new (a, b, c);
  }

  protected virtual void Dispose(bool disposing)
  {
    if (!disposedValue)
    {
      if (disposing)
      {
      }


      a.Dispose();
      b.Dispose();
      c.Dispose();

      disposedValue = true;
    }
  }

 ~MyClass()
  {
    Dispose(disposing: false);
  }

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

I have a class MyClass that requires instances of A, B and C to be created for initialization. All three classes inherit IDisposable. I do not have access to the source code of A, B and C, but their constructors may throw an exception for certain parameter combinations. I know that throwing an exception in a constructor is bad practice because it may leave the class in an inconsistent state in memory (invariant violation). So instead of using a direct constructor, I use a factory method MyClass.Create() that initializes A, B and C inside. The Create function throws an exception if any of A, B and C throws an exception. I know that if I fail to create B, I should at least free A and raise the exception. Also I know that if I fail to create class C, I should at least free previously created A and B, and then raise exception in Create function.

Please tell me, is there a good pattern or code example, how I can clean up previously initialized parts in case when I fail to build class MyClass? What if there are more than three parts? I am thinking about using Stack<IDisposable> stack to clean up parts.

class A: IDisposable
{
  private readonly int p_value;

  public A(int value)
  {
    ArgumentOutOfRangeException.ThrowIfNegative(value);
    p_value = value;
  }

  public void Print()
  {
    Console.WriteLine($"Value is: {p_value}");
  }

  public void Dispose()
  {
    Console.WriteLine("A disposed");
  }
}


class B : IDisposable
{
  private readonly int p_value;

  public B(int value)
  {
    ArgumentOutOfRangeException.ThrowIfLessThan(value, 10);
    p_value = value;
  }

  public void Print()
  {
    Console.WriteLine($"Value is: {p_value}");
  }

  public void Dispose()
  {
    Console.WriteLine("B disposed");
  }
}


class C : IDisposable
{
  private readonly int p_value;

  public C(int value)
  {
    ArgumentOutOfRangeException.ThrowIfLessThan(value, 20);
    p_value = value;
  }

  public void Print()
  {
    Console.WriteLine($"Value is: {p_value}");
  }

  public void Dispose()
  {
    Console.WriteLine("C disposed");
  }
}

class MyClass: IDisposable
{
  private readonly A a;
  private readonly B b;
  private readonly C c;
  private bool disposedValue;

  protected MyClass(A _a, B _b, C _c) { a = _a; b = _b; c = _c;}

  public MyClass Create(int _value)
  {
    // Maybe throw error
    A a = new A(_value);

    // Maybe throw error
    // Need free A
    B b = new B(_value);

    // Maybe throw error
    // Need free A
    // Need free B
    C c = new C(_value);

    return new (a, b, c);
  }

  protected virtual void Dispose(bool disposing)
  {
    if (!disposedValue)
    {
      if (disposing)
      {
      }


      a.Dispose();
      b.Dispose();
      c.Dispose();

      disposedValue = true;
    }
  }

 ~MyClass()
  {
    Dispose(disposing: false);
  }

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

• c#

• 1 How is throwing exceptions in a constructor "bad practice"? If the constructor throws, you don't end up creating a class instance, and you don't violate any "invariants" because there is no class instance in the first place. – Sweeper Commented Feb 16 at 14:21
• @Sweeper If class A holds large unmanaged resources, it will only be released after the GC has run. While it would be most efficient to call Dispose() immediately when it is no longer needed. – Aycon Commented Feb 16 at 14:24
• Are you explicitly designing MyClass for inheritance? That seem to contradict your use of a factory method, and it is generally recommended to only design for inheritance if you have a specific purpose in mind. – JonasH Commented Feb 17 at 7:28

1 Answer 1

You can still use a normal constructor, or a Create function. The key is to use a catch to dispose objects that were created. Note that a constructor that throws doesn't create an object, and therefore is expected to clean up after itself.

public MyClass(int _value)
{
    try
    {
        _a = new A(_value);
        _b = new B(_value);
        _c = new C(_value);
    }
    catch
    {
        _a?.Dispose();
        _b?.Dispose();
        _c?.Dispose();    // not strictly necessary if there is no other code
        GC.SuppressFinalize(this);
        // rethrow original exception
        throw;
    }
}