Nunca haga que los miembros públicos sean virtuales / abstractos, ¿en serio?

20

En la década de 2000, un colega mío me dijo que es un antipatrón hacer que los métodos públicos sean virtuales o abstractos.

Por ejemplo, consideró que una clase como esta no está bien diseñada:

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
  }
}

Él dijo que

  • El desarrollador de una clase derivada que implementa Method1y anula Method2tiene que repetir la validación del argumento.
  • en caso de que el desarrollador de la clase base decida agregar algo alrededor de la parte personalizable Method1o Method2posterior, no puede hacerlo.

En cambio, mi colega propuso este enfoque:

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
  }
}

Me dijo que hacer que los métodos públicos (o propiedades) sean virtuales o abstractos es tan malo como hacer públicos los campos. Al envolver los campos en propiedades, uno puede interceptar cualquier acceso a esos campos más adelante, si es necesario. Lo mismo se aplica a los miembros públicos virtuales / abstractos: envolverlos de la manera que se muestra en la ProtectedAbstractOrVirtualclase permite al desarrollador de la clase base interceptar cualquier llamada que vaya a los métodos virtuales / abstractos.

Pero no veo esto como una guía de diseño. Incluso Microsoft no lo sigue: solo eche un vistazo a la Streamclase para verificar esto.

¿Qué opinas de esa guía? ¿Tiene algún sentido, o crees que está complicando demasiado la API?

Peter Perot
fuente
55
Hacer métodos virtual permite la anulación opcional. Su método probablemente debería ser público, ya que podría no ser anulado. Hacer métodos abstract te obliga a anularlos; probablemente deberían serlo protected, porque no son particularmente útiles en un publiccontexto.
Robert Harvey
44
En realidad, protectedes más útil cuando desea exponer miembros privados de la clase abstracta a clases derivadas. En cualquier caso, no estoy particularmente preocupado por la opinión de tu amigo; elija el modificador de acceso que tenga más sentido para su situación particular.
Robert Harvey
44
Su colega estaba abogando por el patrón de método de plantilla . Puede haber casos de uso para ambas formas, dependiendo de cuán interdependientes sean los dos métodos.
Greg Burghardt
8
@GregBurghardt: parece que el colega del OP siempre sugiere usar el patrón del método de la plantilla, independientemente de si es necesario o no. Ese es un uso excesivo típico de los patrones: si uno tiene un martillo, tarde o temprano cada problema comienza a parecerse a un clavo ;-)
Doc Brown,
3
@PeterPerot: nunca tuve un problema para comenzar con DTO simples con solo campos públicos, y cuando un DTO de ese tipo requiriera miembros con lógica empresarial, refactorícelos a clases con propiedades. Seguramente las cosas son diferentes cuando uno trabaja como vendedor de una biblioteca y tiene que preocuparse por no cambiar las API públicas, incluso incluso convertir un campo público en una propiedad pública del mismo nombre puede causar problemas.
Doc Brown

Respuestas:

30

Diciendo

que es un antipatrón hacer que los métodos públicos sean virtuales o abstractos debido a que el desarrollador de una clase derivada que implementa el Método 1 y anula el Método 2 tiene que repetir la validación del argumento

está mezclando causa y efecto. Supone que cada método reemplazable requiere una validación de argumento no personalizable. Pero es todo lo contrario:

Si se quiere diseñar un método de manera que proporcione algunas validaciones de argumentos fijos en todas las derivaciones de la clase (o, más en general, una parte personalizable y una parte no personalizable), entonces tiene sentido hacer que el punto de entrada no sea virtual y, en su lugar, proporcione un método virtual o abstracto para la parte personalizable que se llama internamente.

Pero hay un montón de ejemplos en los que tiene sentido perfecto para tener un método virtual pública, ya que no se fija no personalizable parte: vistazo a los métodos estándar como ToStringo Equalso GetHashCode- habría que mejorar el diseño de la objectclase para tener estos no pública y virtual al mismo tiempo? No lo creo.

O, en términos de su propio código: cuando el código en la clase base finalmente e intencionalmente se ve así

 public void Method1(string argument)
 {
    // nothing to validate here, all strings including null allowed
    this.Method1Core(argument);
 }

tener esta separación entre Method1y Method1Coresolo complica las cosas sin razón aparente.

Doc Brown
fuente
1
En el caso del ToString()método, Microsoft mejor lo hizo no virtual e introdujo un método de plantilla virtual ToStringCore(). Por qué: debido a esto: ToString()- nota para los herederos . Afirman que ToString()no debe devolver nulo. Podrían haber coaccionado esta demanda mediante la implementación ToString() => ToStringCore() ?? string.Empty.
Peter Perot
99
@PeterPerot: esa guía a la que se vinculó también recomienda no regresar string.Empty, ¿lo notó? Y recomienda muchas otras cosas que no pueden aplicarse en el código introduciendo algo así como un ToStringCoremétodo. Entonces, esta técnica probablemente no sea la herramienta adecuada para ToString.
Doc Brown
3
@Theraot: seguramente uno puede encontrar algunas razones o argumentos por los que ToString and Equals o GetHashcode podrían haber sido diseñados de manera diferente, pero hoy son como son (al menos creo que su diseño es lo suficientemente bueno como para dar un buen ejemplo).
Doc Brown
1
@PeterPerot "Vi muchos códigos como en anyObjectIGot.ToString()lugar de anyObjectIGot?.ToString()": ¿cómo es esto relevante? Su ToStringCore()enfoque evitaría que se devuelva una cadena nula, pero aún arrojaría un NullReferenceExceptionsi el objeto es nulo.
Miil
1
@PeterPerot No estaba tratando de transmitir un argumento de la autoridad. No es tanto lo que usa Microsoft public virtual, sino que hay casos en los que public virtualestá bien. Podríamos argumentar a favor de una parte vacía no personalizable como una prueba futura del código ... Pero eso no funciona. Retroceder y cambiarlo podría romper los tipos derivados. Por lo tanto, no gana nada.
Theraot
6

Hacerlo de la manera que sugiere su colega proporciona más flexibilidad al implementador de la clase base. Pero con esto también viene una mayor complejidad que generalmente no se justifica por los supuestos beneficios.

Tenga en cuenta que la mayor flexibilidad para el implementador de la clase base se produce a expensas de una menor flexibilidad para la parte primordial. Obtienen un comportamiento impuesto que quizás no les interese particularmente. Para ellos, las cosas se han vuelto más rígidas. Esto puede ser justificado y útil, pero todo depende del escenario.

La convención de nomenclatura para implementar esto (que yo sepa) es reservar el buen nombre para la interfaz pública y prefijar el nombre del método interno con "Do".

Un caso útil es cuando la acción realizada necesita algo de configuración y algo de cierre. Al igual que abrir una secuencia y cerrarla después de que la anulación haya terminado. En general, el mismo tipo de inicialización y finalización. Es un patrón válido para usar, pero sería inútil ordenar su uso en todos los escenarios abstractos y virtuales.

Martin Maat
fuente
1
El prefijo del método Do es una opción. Microsoft a menudo usa el método básico Postfix.
Peter Perot
@Peter Perot. Nunca vi el prefijo Core en ningún material de Microsoft, pero esto puede deberse a que últimamente no he prestado mucha atención. Sospecho que comenzaron a hacer esto recientemente solo para promocionar el apodo Core, para hacerse un nombre para .NET Core.
Martin Maat
No, es un viejo sombrero: BindingList. Además, encontré la recomendación en alguna parte, tal vez sus Pautas de diseño del marco o algo similar. Y: es un postfix . ;-)
Peter Perot
Menos flexibilidad para la clase derivada es el punto. La clase base es un límite de abstracción. La clase base le dice al consumidor lo que hace su API pública, y define la API que se requiere para lograr esos objetivos. Si una clase derivada puede anular un método público en la clase base, corre un mayor riesgo de violar el Principio de sustitución de Liskov.
Adrian McCarthy
2

En C ++, esto se denomina patrón de interfaz no virtual (NVI). (Érase una vez, se llamaba el Método de la Plantilla. Eso era confuso, pero algunos de los artículos más antiguos tienen esa terminología). Herb Sutter promueve el NVI, quien ha escrito sobre él al menos algunas veces. Creo que uno de los primeros está aquí .

Si recuerdo correctamente, la premisa es que una clase derivada no debería cambiar lo que hace la clase base, sino cómo lo hace.

Por ejemplo, una Forma podría tener un método Mover para reubicar la forma. Las implementaciones concretas (p. Ej., Como el cuadrado y los círculos) no deberían anular directamente el movimiento, porque Shape define lo que significa mover (a nivel conceptual). Un cuadrado puede tener detalles de implementación diferentes que un círculo en términos de cómo se representa internamente la posición, por lo que tendrá que anular algún método para proporcionar la funcionalidad Mover.

En ejemplos simples, esto a menudo se reduce a un Move público que simplemente delega todo el trabajo en un ReallyDoTheMove virtual privado, por lo que parece una gran sobrecarga sin ningún beneficio.

Pero esta correspondencia uno a uno no es un requisito. Por ejemplo, podría agregar un método Animate a la API pública de Shape, y podría implementarlo llamando a ReallyDoTheMove en un bucle. Termina con dos API de métodos públicos no virtuales que dependen del método abstracto privado. Sus círculos y cuadrados no necesitan hacer ningún trabajo adicional, ni tampoco puede anular Animate .

La clase base define la interfaz pública utilizada por los consumidores, y define una interfaz de operaciones primitivas que necesita para implementar esos métodos públicos. Los tipos derivados son responsables de proporcionar implementaciones de esas operaciones primitivas.

No conozco ninguna diferencia entre C # y C ++ que pueda cambiar este aspecto del diseño de clase.

Adrian McCarthy
fuente
¡Buen descubrimiento! Ahora recuerdo que encontré exactamente esa publicación a la que apunta tu segundo enlace (o una copia), en la década de 2000. Recuerdo que estaba buscando más evidencia de la afirmación de mi colega y que no encontré nada en el contexto de C #, sino C ++. Esta. Es. ¡Eso! :-) Pero, de vuelta en C # land, parece que este patrón no se usa mucho. Tal vez la gente se dio cuenta de que agregar la funcionalidad base más tarde también puede romper las clases derivadas, y que el uso estricto de TMP o NVIP en lugar de métodos virtuales públicos no siempre tiene sentido.
Peter Perot
Nota personal: es bueno saber que este patrón tiene un nombre: NVIP.
Peter Perot