Skip to main content
Tweeted twitter.com/StackSoftEng/status/1170939906216071170
deleted 2 characters in body
Source Link

Back in the 2000ies2000s a colleague of mine told me that it is an anti-pattern to make public methods virtual or abstract.

For example, he considered a class like this not well designed:

public abstract class PublicAbstractOrVirtual
{
  public abstract void Method1(string argument);

  public virtual void Method2(string argument)
  {
    if (argument == null) throw new ArgumentNullException(nameof(argument));
    // default implementation
  }
}

He stated that

  • the developer of a derived class that implements Method1 and overrides Method2 has to repeat the argument validation.
  • in case the developer of the base class decides to add something around the customizable part of Method1 or Method2 later, he cannot do it.

Instead my colleague proposed this approach:

public abstract class ProtectedAbstractOrVirtual
{
  public void Method1(string argument)
  {
    if (argument == null) throw new ArgumentNullException(nameof(argument));
    this.Method1Core(argument);
  }

  public void Method2(string argument)
  {
    if (argument == null) throw new ArgumentNullException(nameof(argument));
    this.Method2Core(argument);
  }

  protected abstract void Method1Core(string argument);

  protected virtual void Method2Core(string argument)
  {
    // default implementation
  }
}

He told me making public methods (or properties) virtual or abstract is as bad as making fields public. By wrapping fields into properties one can intercept any access to that fields later, if needed. The same applies to public virtual/abstract members: wrapping them the way as shown in the ProtectedAbstractOrVirtual class allows the base class developer to intercept any calls that go to the virtual/abstract methods.

But I don't see this as a design guideline. Even Microsoft doesn't follow it: just have a look at the Stream class to verify this.

What do you think of that guidline? Does it make any sense, or do you think it's overcomplicating the API?

Back in the 2000ies a colleague of mine told me that it is an anti-pattern to make public methods virtual or abstract.

For example, he considered a class like this not well designed:

public abstract class PublicAbstractOrVirtual
{
  public abstract void Method1(string argument);

  public virtual void Method2(string argument)
  {
    if (argument == null) throw new ArgumentNullException(nameof(argument));
    // default implementation
  }
}

He stated that

  • the developer of a derived class that implements Method1 and overrides Method2 has to repeat the argument validation.
  • in case the developer of the base class decides to add something around the customizable part of Method1 or Method2 later, he cannot do it.

Instead my colleague proposed this approach:

public abstract class ProtectedAbstractOrVirtual
{
  public void Method1(string argument)
  {
    if (argument == null) throw new ArgumentNullException(nameof(argument));
    this.Method1Core(argument);
  }

  public void Method2(string argument)
  {
    if (argument == null) throw new ArgumentNullException(nameof(argument));
    this.Method2Core(argument);
  }

  protected abstract void Method1Core(string argument);

  protected virtual void Method2Core(string argument)
  {
    // default implementation
  }
}

He told me making public methods (or properties) virtual or abstract is as bad as making fields public. By wrapping fields into properties one can intercept any access to that fields later, if needed. The same applies to public virtual/abstract members: wrapping them the way as shown in the ProtectedAbstractOrVirtual class allows the base class developer to intercept any calls that go to the virtual/abstract methods.

But I don't see this as a design guideline. Even Microsoft doesn't follow it: just have a look at the Stream class to verify this.

What do you think of that guidline? Does it make any sense, or do you think it's overcomplicating the API?

Back in the 2000s a colleague of mine told me that it is an anti-pattern to make public methods virtual or abstract.

For example, he considered a class like this not well designed:

public abstract class PublicAbstractOrVirtual
{
  public abstract void Method1(string argument);

  public virtual void Method2(string argument)
  {
    if (argument == null) throw new ArgumentNullException(nameof(argument));
    // default implementation
  }
}

He stated that

  • the developer of a derived class that implements Method1 and overrides Method2 has to repeat the argument validation.
  • in case the developer of the base class decides to add something around the customizable part of Method1 or Method2 later, he cannot do it.

Instead my colleague proposed this approach:

public abstract class ProtectedAbstractOrVirtual
{
  public void Method1(string argument)
  {
    if (argument == null) throw new ArgumentNullException(nameof(argument));
    this.Method1Core(argument);
  }

  public void Method2(string argument)
  {
    if (argument == null) throw new ArgumentNullException(nameof(argument));
    this.Method2Core(argument);
  }

  protected abstract void Method1Core(string argument);

  protected virtual void Method2Core(string argument)
  {
    // default implementation
  }
}

He told me making public methods (or properties) virtual or abstract is as bad as making fields public. By wrapping fields into properties one can intercept any access to that fields later, if needed. The same applies to public virtual/abstract members: wrapping them the way as shown in the ProtectedAbstractOrVirtual class allows the base class developer to intercept any calls that go to the virtual/abstract methods.

But I don't see this as a design guideline. Even Microsoft doesn't follow it: just have a look at the Stream class to verify this.

What do you think of that guidline? Does it make any sense, or do you think it's overcomplicating the API?

Became Hot Network Question
Source Link

Never make public members virtual/abstract - really?

Back in the 2000ies a colleague of mine told me that it is an anti-pattern to make public methods virtual or abstract.

For example, he considered a class like this not well designed:

public abstract class PublicAbstractOrVirtual
{
  public abstract void Method1(string argument);

  public virtual void Method2(string argument)
  {
    if (argument == null) throw new ArgumentNullException(nameof(argument));
    // default implementation
  }
}

He stated that

  • the developer of a derived class that implements Method1 and overrides Method2 has to repeat the argument validation.
  • in case the developer of the base class decides to add something around the customizable part of Method1 or Method2 later, he cannot do it.

Instead my colleague proposed this approach:

public abstract class ProtectedAbstractOrVirtual
{
  public void Method1(string argument)
  {
    if (argument == null) throw new ArgumentNullException(nameof(argument));
    this.Method1Core(argument);
  }

  public void Method2(string argument)
  {
    if (argument == null) throw new ArgumentNullException(nameof(argument));
    this.Method2Core(argument);
  }

  protected abstract void Method1Core(string argument);

  protected virtual void Method2Core(string argument)
  {
    // default implementation
  }
}

He told me making public methods (or properties) virtual or abstract is as bad as making fields public. By wrapping fields into properties one can intercept any access to that fields later, if needed. The same applies to public virtual/abstract members: wrapping them the way as shown in the ProtectedAbstractOrVirtual class allows the base class developer to intercept any calls that go to the virtual/abstract methods.

But I don't see this as a design guideline. Even Microsoft doesn't follow it: just have a look at the Stream class to verify this.

What do you think of that guidline? Does it make any sense, or do you think it's overcomplicating the API?