c# - práctico - ¿Cuál es el nombre de esta mala práctica/antipatrón?
patrones y anti patrones de diseño (14)
Estoy tratando de explicarle a mi equipo por qué esta es una mala práctica, y estoy buscando una referencia antipatrón para ayudar en mi explicación. Esta es una aplicación empresarial muy grande, así que aquí hay un ejemplo simple para ilustrar lo que se implementó:
public void ControlStuff()
{
var listOfThings = LoadThings();
var listOfThingsThatSupportX = new string[] {"ThingA","ThingB", "ThingC"};
foreach (var thing in listOfThings)
{
if(listOfThingsThatSupportX.Contains(thing.Name))
{
DoSomething();
}
}
}
Estoy sugiriendo que agreguemos una propiedad a la clase base ''Cosas'' para decirnos si es compatible con X, ya que la subclase Thing deberá implementar la funcionalidad en cuestión. Algo como esto:
public void ControlStuff()
{
var listOfThings = LoadThings();
foreach (var thing in listOfThings)
{
if (thing.SupportsX)
{
DoSomething();
}
}
}
class ThingBase
{
public virtual bool SupportsX { get { return false; } }
}
class ThingA : ThingBase
{
public override bool SupportsX { get { return true; } }
}
class ThingB : ThingBase
{
}
Entonces, es bastante obvio por qué el primer enfoque es una mala práctica, pero ¿cómo se llama esto? Además, ¿hay un patrón más adecuado para este problema que el que estoy sugiriendo?
Como no muestras en qué consiste realmente el código, es difícil darte un buen tono. Aquí hay uno que no usa ninguna cláusula if .
// invoked to map different kinds of items to different features
public void BootStrap
{
featureService.Register(typeof(MyItem), new CustomFeature());
}
// your code without any ifs.
public void ControlStuff()
{
var listOfThings = LoadThings();
foreach (var thing in listOfThings)
{
thing.InvokeFeatures();
}
}
// your object
interface IItem
{
public ICollection<IFeature> Features {get;set;}
public void InvokeFeatues()
{
foreach (var feature in Features)
feature.Invoke(this);
}
}
// a feature that can be invoked on an item
interface IFeature
{
void Invoke(IItem container);
}
// the "glue"
public class FeatureService
{
void Register(Type itemType, IFeature feature)
{
_features.Add(itemType, feature);
}
void ApplyFeatures<T>(T item) where T : IItem
{
item.Features = _features.FindFor(typof(T));
}
}
Creo que el nombre anti-patrón es difícil de codificar :)
Si debe haber un ThingBase.supportsX depende al menos algo de lo que X es. En casos excepcionales, ese conocimiento podría estar en ControlStuff() solamente.
Sin embargo, más habitualmente, X podría ser uno de los elementos en cuyo caso ThingBase podría necesitar exponer sus capacidades usando ThingBase.supports(ThingBaseProperty) o algo así.
El exceso de código de escritura antipatrón. Hace que sea más difícil de leer y entender.
Como ya se señaló, sería mejor usar una interfaz.
Básicamente, los programadores no se están aprovechando de los Principios orientados a objetos y, en su lugar, están haciendo cosas usando el código de procedimiento. Cada vez que buscamos la afirmación ''si'' deberíamos preguntarnos si no deberíamos usar un concepto OO en lugar de escribir más código de procedimiento.
En lugar de utilizar interfaces, puede usar atributos. Probablemente describan que el objeto debería estar ''etiquetado'' como este tipo de objeto, incluso si etiquetarlo como tal no introduce ninguna funcionalidad adicional. Es decir, un objeto que se describe como "Cosa A" no significa que todas las "Cosas A" tengan una interfaz específica, es importante que sean "Cosa A". Parece más el trabajo de los atributos que las interfaces.
Es solo un código incorrecto, no tiene nombre (ni siquiera tiene un diseño OO). Pero el argumento podría ser que el primer código no está en barbecho Open Close Principle . ¿Qué sucede cuando cambia la lista de cosas compatibles? Tienes que volver a escribir el método que estás usando.
Pero sucede lo mismo cuando usa el segundo fragmento de código. Digamos que la regla de soporte cambia, tendrías que ir a cada uno de los métodos y reescribirlos. Le sugiero que tenga una clase de soporte abstracta y apruebe diferentes reglas de soporte cuando cambien.
Hay una situación perfectamente razonable en la que esta práctica de codificación tiene sentido. Puede que no sea un problema el hecho de que las cosas sean compatibles con X (donde, por supuesto, sería mejor una interfaz para cada cosa), sino más bien con las cosas que son compatibles con X y que desea habilitar . La etiqueta para lo que ves es simplemente configuración , actualmente codificada , y la mejora en esto es moverla eventualmente a un archivo de configuración o de lo contrario. Antes de persuadir a su equipo para que lo modifique, comprobaría que esta no es la intención del código que ha parafraseado.
No creo que tenga un nombre, pero tal vez consulte la lista maestra en http://en.wikipedia.org/wiki/Anti-pattern sabe? http://en.wikipedia.org/wiki/Hard_code probablemente se ve más de cerca.
Creo que su ejemplo probablemente no tenga un nombre, mientras que su solución propuesta se llama Composite .
No sé si hay un "patrón" para escribir código que no se puede mantener o reutilizar. ¿Por qué no puedes darles la razón?
No sé sobre un nombre (la duda existe) pero piense en cada "Cosa" como un automóvil: algunos autos tienen un sistema de control de crucero y otros no.
Ahora tiene una flota de autos que administra y desea saber que tienen control de crucero.
Usar el primer enfoque es como encontrar la lista de todos los modelos de automóviles que tienen control de crucero, luego ir en automóvil y buscar cada uno en esa lista; si eso significa que el automóvil tiene control de crucero, de lo contrario no tiene. Engorroso, ¿verdad?
Usar el segundo enfoque significa que cada automóvil que tiene control de crucero viene con una pegatina que dice "Tengo control de crucero" y solo tiene que buscar esa pegatina, sin depender de una fuente externa para brindarle información.
Explicación no muy técnica, pero simple y al grano.
Normalmente, un mejor enfoque (IMHO) sería utilizar interfaces en lugar de herencia
entonces solo es cuestión de verificar si el objeto ha implementado la interfaz o no.
OMI, el principio fundamental de diseño en juego aquí es la encapsulación. En la solución propuesta, ha encapsulado la lógica dentro de la clase Thing, donde, como en el código original, la lógica se filtra a las personas que llaman.
También viola el principio de Open-Closed, ya que si desea agregar nuevas subclases que admitan X, ahora necesita ir y modificar cualquier lugar que contenga esa lista codificada. Con su solución, simplemente agrega la nueva clase, anula el método y listo.
Para mí, lo mejor es explicar eso en términos de complejidad computacional. Dibuje dos tablas que muestren el número de operaciones requeridas en términos de count(listOfThingsThatSupportX ) y count(listOfThings ) y compárelas con la solución que usted propone.
Si fuera una cadena, podría llamarla "cadena mágica". En este caso, consideraría "matriz de cadenas mágicas".
Yo lo llamaría un Failure to Encapsulate . Es un término inventado, pero es real y se ve con bastante frecuencia
Muchas personas olvidan que la encasillamiento no es solo el ocultamiento de datos dentro de un objeto, sino también el ocultamiento del comportamiento dentro de ese objeto, o más específicamente, el ocultamiento de cómo se implementa el comportamiento de un objeto.
Al tener un DoSomething() externo DoSomething() , que se requiere para la operación correcta del programa, crea muchos problemas. No puede usar razonablemente la herencia en su lista de cosas. Si cambia la firma de la "cosa", en este caso la cadena, el comportamiento no sigue. Necesita modificar esta clase externa para agregar su comportamiento (invocando DoSomething() nuevo a la thing derivada.
DoSomething() solución "mejorada", que es tener una lista de objetos Thing , con un método que implementa DoSomething() , que actúa como un NOOP para las cosas que no hacen nada. Esto localiza el comportamiento de la thing dentro de sí mismo, y el mantenimiento de una lista de correspondencia especial se vuelve innecesaria.