¿Cuándo las enumeraciones NO huelen a código?

17

Dilema

He estado leyendo muchos libros de mejores prácticas sobre prácticas orientadas a objetos, y casi todos los libros que he leído tienen una parte donde dicen que las enumeraciones son un olor a código. Creo que se han perdido la parte en la que explican cuándo las enumeraciones son válidas.

Como tal, estoy buscando pautas y / o casos de uso donde las enumeraciones NO son un olor a código y, de hecho, una construcción válida.

Fuentes:

"ADVERTENCIA Como regla general, las enumeraciones son olores de código y deben refactorizarse a clases polimórficas. [8]" Seemann, Mark, Dependency Injection en .Net, 2011, p. 342

[8] Martin Fowler et al., Refactoring: Improving the Design of Existing Code (Nueva York: Addison-Wesley, 1999), 82.

Contexto

La causa de mi dilema es una API comercial. Me dan una secuencia de datos de Tick enviándome a través de este método:

void TickPrice(TickType tickType, double value)

dónde enum TickType { BuyPrice, BuyQuantity, LastPrice, LastQuantity, ... }

Intenté hacer un envoltorio alrededor de esta API porque romper los cambios es la forma de vida de esta API. Quería hacer un seguimiento del valor de cada último tipo de tick recibido en mi contenedor y lo hice usando un Diccionario de tipos de tick:

Dictionary<TickType,double> LastValues

Para mí, esto parecía un uso adecuado de una enumeración si se usan como claves. Pero estoy teniendo dudas porque tengo un lugar donde tomo una decisión basada en esta colección y no puedo pensar en cómo podría eliminar la declaración de cambio, podría usar una fábrica, pero esa fábrica aún tendrá un cambiar la declaración en alguna parte. Me pareció que solo estaba moviendo cosas, pero todavía huele.

Es fácil encontrar los DON'T de las enumeraciones, pero los DO no son tan fáciles, y agradecería que las personas puedan compartir su experiencia, los pros y los contras.

Segundos pensamientos

Algunas decisiones y acciones se basan en estos TickTypey parece que no puedo pensar en una forma de eliminar las declaraciones enum / switch. La solución más limpia que se me ocurre es usar una fábrica y devolver una implementación basada en TickType. Aún así, tendré una declaración de cambio que devuelve una implementación de una interfaz.

A continuación se enumera una de las clases de muestra donde tengo dudas de que podría estar usando una enumeración incorrecta:

public class ExecutionSimulator
{
  Dictionary<TickType, double> LastReceived;
  void ProcessTick(TickType tickType, double value)
  {
    //Store Last Received TickType value
    LastReceived[tickType] = value;

    //Perform Order matching only on specific TickTypes
    switch(tickType)
    {
      case BidPrice:
      case BidSize:
        MatchSellOrders();
        break;
      case AskPrice:
      case AskSize:
        MatchBuyOrders();
        break;
    }       
  }
}
Stromms
fuente
25
Nunca he oído hablar de que las enumeraciones sean un olor a código. ¿Podría incluir una referencia? Creo que tienen mucho sentido para un número limitado de valores potenciales
Richard Tingle
24
¿Qué libros dicen que las enumeraciones son un olor a código? Consigue mejores libros.
JacquesB
33
Entonces la respuesta a la pregunta sería "la mayor parte del tiempo".
gnasher729
55
¿Un buen valor de tipo seguro que transmite significado? ¿Eso garantiza que las claves adecuadas se utilizan en un diccionario? ¿Cuándo es un olor a código?
77
Sin contexto, creo que una mejor redacción seríaenums as switch statements might be a code smell ...
pllee

Respuestas:

31

Las enumeraciones están destinadas a casos de uso cuando literalmente ha enumerado todos los valores posibles que una variable podría tomar. Nunca. Piense en casos de uso como días de la semana o meses del año o valores de configuración de un registro de hardware. Cosas que son altamente estables y representables por un valor simple.

Tenga en cuenta que si está creando una capa anticorrupción, no puede evitar tener una declaración de cambio en alguna parte , debido al diseño que está envolviendo, pero si lo hace bien, puede limitarlo a ese lugar y usar polimorfismo en otro lugar.

Karl Bielefeldt
fuente
33
Este es el punto más importante: agregar un valor adicional a una enumeración significa encontrar cada uso del tipo en su código y potencialmente necesitar agregar una nueva rama para el nuevo valor. Compare con un tipo polimórfico, donde agregar una nueva posibilidad simplemente significa crear una nueva clase que implemente la interfaz: los cambios se concentran en un solo lugar, por lo que es más fácil de hacer. Si está seguro de que nunca se agregará un nuevo tipo de marca, la enumeración está bien, de lo contrario, debe estar envuelta en una interfaz polimórfica.
Jules
Esa es una de las respuestas que estaba buscando. Simplemente no puedo evitarlo cuando el método que estoy envolviendo usa una enumeración y el objetivo es centralizar estas enumeraciones en un solo lugar. Tengo que hacer el contenedor de tal manera que si alguna vez la API cambia estas enumeraciones, solo tengo que cambiar el contenedor, y no los consumidores del contenedor.
Stromms
@Jules: "Si está seguro de que nunca se agregará un nuevo tipo de marca ..." Esa afirmación es incorrecta en muchos niveles. Una clase para cada tipo individual, a pesar de que cada tipo tiene exactamente el mismo comportamiento, es lo más lejos posible de un enfoque "razonable". No es hasta que tenga comportamientos diferentes y comience a necesitar declaraciones "switch / if-then" cuando la línea debe trazarse. Ciertamente no se basa en si podría agregar o no más valores de enumeración en el futuro.
Dunk
@dunk Creo que entendiste mal mi comentario. Nunca sugerí crear varios tipos con un comportamiento idéntico, eso sería una locura.
Jules
1
Incluso si ha "enumerado todos los valores posibles", eso no significa que el tipo nunca tendrá una funcionalidad adicional por instancia. Incluso algo como Mes puede tener otras propiedades útiles, como el número de días. El uso de una enumeración evita inmediatamente que se amplíe el concepto que está modelando. Básicamente es un mecanismo / patrón anti-OOP.
Dave Cousineau
17

En primer lugar, un olor a código no significa que algo esté mal. Significa que algo podría estar mal. enumhuele porque se abusa con frecuencia, pero eso no significa que tengas que evitarlos. Simplemente te encuentras escribiendo enum, detente y busca mejores soluciones.

El caso particular que ocurre con mayor frecuencia es cuando las diferentes enumeraciones corresponden a diferentes tipos con diferentes comportamientos pero la misma interfaz. Por ejemplo, hablar con diferentes backends, renderizar diferentes páginas, etc. Estos se implementan mucho más naturalmente usando clases polimórficas.

En su caso, el TickType no corresponde a comportamientos diferentes. Son diferentes tipos de eventos o diferentes propiedades del estado actual. Así que creo que este es el lugar ideal para una enumeración.

Winston Ewert
fuente
¿Está diciendo que cuando las enumeraciones contienen sustantivos como entradas (por ejemplo, colores), probablemente se usan correctamente. ¿Y cuando son verbos (conectados, renderizados) entonces es una señal de que se está usando mal?
Stromms
2
@stromms, la gramática es una guía terrible para la arquitectura de software. Si TickType fuera una interfaz en lugar de una enumeración, ¿cuáles serían los métodos? No puedo ver ninguno, por eso me parece un caso de enumeración. En otros casos, como el estado, los colores, el backend, etc., puedo encontrar métodos, y por eso son interfaces.
Winston Ewert el
@ WinstonEwert y con la edición, parece que son comportamientos diferentes.
Ohhh Entonces, si puedo pensar en un método para una enumeración, ¿podría ser un candidato para una interfaz? Ese podría ser un muy buen punto.
Stromms
1
@stromms, ¿puede compartir más ejemplos de interruptores, para que pueda tener una mejor idea de cómo usa TickType?
Winston Ewert
8

Cuando se transmiten datos, las enumeraciones no huelen a código

En mi humilde opinión, cuando se transmiten datos utilizando enumspara indicar que un campo puede tener un valor de un conjunto restringido (rara vez cambia) de valores es bueno.

Considero preferible transmitir arbitrariamente stringso ints. Las cadenas pueden causar problemas por variaciones en la ortografía y las mayúsculas. Permite transmitir valores fuera de rango y tener poca semántica (por ejemplo, recibo 3de su servicio comercial, ¿qué significa? LastPrice¿ LastQuantity? ¿Algo más?

Transmitir objetos y usar jerarquías de clases no siempre es posible; por ejemplo, no permite que el extremo receptor distinga qué clase se ha enviado.

En mi proyecto, el servicio utiliza una jerarquía de clases para los efectos de las operaciones, justo antes de transmitir a través de un DataContractobjeto de la jerarquía de clases se copia en un unionobjeto similar que contiene un enumpara indicar el tipo. El cliente recibe DataContracty crea un objeto de una clase en una jerarquía utilizando el enumvalor ay switchcrea un objeto del tipo correcto.

Otra razón por la que uno no querría transmitir objetos de clase es que el servicio puede requerir un comportamiento completamente diferente para un objeto transmitido (como LastPrice) que el cliente. En ese caso, el envío de la clase y sus métodos no es deseado.

¿Son malas las declaraciones de cambio?

En mi humilde opinión, una declaración única switch que llama a diferentes constructores dependiendo de un enumno es un código de olor. No es necesariamente mejor o peor que otros métodos, como la base de reflexión en un nombre de tipo; Esto depende de la situación real.

Tener interruptores en enumtodo el lugar es un olor a código, ofrece alternativas que a menudo son mejores:

  • Usar objetos de diferentes clases de una jerarquía que tienen métodos anulados; es decir, polimorfismo.
  • Use el patrón de visitante cuando las clases en la jerarquía rara vez cambien y cuando las (muchas) operaciones se deban unir a las clases en la jerarquía.
Kasper van den Berg
fuente
En realidad, TickType se transmite a través del cable como un int. Mi contenedor es el que usa el TickTypeque se emite desde el recibido int. Varios eventos usan este ticktype con firmas variables que responden a varias solicitudes. ¿Es una práctica común tener un uso intcontinuo para diferentes funciones? por ejemplo, TickPrice(int type, double value)utiliza 1,3 y 6 para typemientras TickSize(int type, double valueusa 2,4 y 5? ¿Tiene sentido separarlos en dos eventos?
Stromms
2

¿Qué pasa si fue con un tipo más complejo:

    abstract class TickType
    {
      public abstract string Name {get;}
      public abstract double TickValue {get;}
    }

    class BuyPrice : TickType
    {
      public override string Name { get { return "Buy Price"; } }
      public override double TickValue { get { return 2.35d; } }
    }

    class BuyQuantity : TickType
    {
      public override string Name { get { return "Buy Quantity"; } }
      public override double TickValue { get { return 4.55d; } }
    }
//etcetera

entonces podría cargar sus tipos desde la reflexión o construirlo usted mismo, pero lo más importante aquí es que está manteniendo abierto el principio de SOLID

Chris
fuente
1

TL; DR

Para responder a la pregunta, ahora estoy teniendo dificultades para pensar en un momento en que las enumeraciones no son un olor a código en algún nivel. Hay una cierta intención que declaran eficientemente (hay un número limitado de posibilidades claramente limitadas para este valor), pero su naturaleza inherentemente cerrada los hace arquitectónicamente inferiores.

Disculpe mientras refactorizo ​​toneladas de mi código heredado. / suspiro; ^ D


¿Pero por qué?

Muy buena revisión por qué ese es el caso de LosTechies aquí :

// calculate the service fee
public double CalculateServiceFeeUsingEnum(Account acct)
{
    double totalFee = 0;
    foreach (var service in acct.ServiceEnums) { 

        switch (service)
        {
            case ServiceTypeEnum.ServiceA:
                totalFee += acct.NumOfUsers * 5;
                break;
            case ServiceTypeEnum.ServiceB:
                totalFee += 10;
                break;
        }
    }
    return totalFee;
} 

Esto tiene los mismos problemas que el código anterior. A medida que la aplicación se hace más grande, las posibilidades de tener declaraciones de sucursal similares van a aumentar. Además, a medida que despliegue más servicios premium, tendrá que modificar continuamente este código, lo que viola el Principio de Abierto-Cerrado. Hay otros problemas aquí también. La función para calcular la tarifa de servicio no debería necesitar conocer las cantidades reales de cada servicio. Esa es información que necesita ser encapsulada.

Un pequeño aparte: las enumeraciones son una estructura de datos muy limitada. Si no está utilizando una enumeración para lo que realmente es, un entero etiquetado, necesita una clase para modelar realmente la abstracción correctamente. Puedes usar la increíble clase de Enumeración de Jimmy para usar clases que también nos usen como etiquetas.

Refactoricemos esto para usar el comportamiento polimórfico. Lo que necesitamos es abstracción que nos permita contener el comportamiento necesario para calcular la tarifa de un servicio.

public interface ICalculateServiceFee
{
    double CalculateServiceFee(Account acct);
}

...

Ahora podemos crear nuestras implementaciones concretas de la interfaz y adjuntarlas [a] la cuenta.

public class Account{
    public int NumOfUsers{get;set;}
    public ICalculateServiceFee[] Services { get; set; }
} 

public class ServiceA : ICalculateServiceFee
{
    double feePerUser = 5; 

    public double CalculateServiceFee(Account acct)
    {
        return acct.NumOfUsers * feePerUser;
    }
} 

public class ServiceB : ICalculateServiceFee
{
    double serviceFee = 10;
    public double CalculateServiceFee(Account acct)
    {
        return serviceFee;
    }
} 

Otro caso de implementación ...

La conclusión es que si tiene un comportamiento que depende de los valores de enumeración, ¿por qué no tener implementaciones diferentes de una interfaz similar o una clase principal que garantice la existencia de ese valor? En mi caso, estoy viendo diferentes mensajes de error basados ​​en códigos de estado REST. En lugar de...

private static string _getErrorCKey(int statusCode)
{
    string ret;

    switch (statusCode)
    {
        case StatusCodes.Status403Forbidden:
            ret = "BRANCH_UNAUTHORIZED";
            break;

        case StatusCodes.Status422UnprocessableEntity:
            ret = "BRANCH_NOT_FOUND";
            break;

        default:
            ret = "BRANCH_GENERIC_ERROR";
            break;
    }

    return ret;
}

... quizás debería envolver los códigos de estado en las clases.

public interface IAmStatusResult
{
    int StatusResult { get; }    // Pretend an int's okay for now.
    string ErrorKey { get; }
}

Luego, cada vez que necesito un nuevo tipo de IAmStatusResult, lo codifico ...

public class UnauthorizedBranchStatusResult : IAmStatusResult
{
    public int StatusResult => 403;
    public string ErrorKey => "BRANCH_UNAUTHORIZED";
}

... y ahora puedo asegurarme de que el código anterior se da cuenta de que tiene un IAmStatusResultalcance y hacer referencia a su en entity.ErrorKeylugar del más complicado, deadend _getErrorCode(403).

Y, lo que es más importante, cada vez que agrego un nuevo tipo de valor de retorno, no es necesario agregar ningún otro código para manejarlo . Whaddya saber, el enumy switchera probable código de olores.

Lucro.

ruffin
fuente
¿Qué tal el caso en que, por ejemplo, yo tengo clases polimórficas y quiero configurar cuál estoy usando a través de un parámetro de línea de comandos? La línea de comando -> enum -> switch / map -> implementación parece un caso de uso bastante legítimo. Creo que la clave es que la enumeración se usa para la orquestación, no como una implementación de lógica condicional.
Ant P
@AntP ¿Por qué no, en este caso, usar el StatusResultvalor? Se podría argumentar que la enumeración es útil como un atajo memorable para los humanos en ese caso de uso, pero probablemente todavía lo llamaría un olor a código, ya que hay buenas alternativas que no requieren la colección cerrada.
ruffin
Que alternativas ¿Una cuerda? Esa es una distinción arbitraria desde la perspectiva de "las enumeraciones deben ser reemplazadas por polimorfismo": cualquiera de los dos enfoques requiere asignar un valor a un tipo; evitando la enumeración en este caso no se logra nada.
Ant P
@AntP No estoy seguro de dónde cruzan los cables aquí. Si hace referencia a una propiedad en una interfaz, puede colocar esa interfaz en cualquier lugar que sea necesario y nunca actualizar ese código cuando crea una implementación nueva (o elimina una existente) de esa interfaz . Si usted tiene una enum, usted no tiene que actualizar el código cada vez que se añade o elimina un valor, y potencialmente una gran cantidad de lugares, dependiendo de cómo se estructurado su código. Usar polimorfismo en lugar de una enumeración es como asegurarse de que su código esté en forma normal de Boyce-Codd .
ruffin
Si. Ahora suponga que tiene N implementaciones polimórficas de este tipo. En el artículo de Los Techies, simplemente están iterando a través de todos ellos. Pero, ¿qué sucede si quiero aplicar condicionalmente solo una implementación basada, por ejemplo, en parámetros de línea de comandos? Ahora debemos definir una asignación de algún valor de configuración a algún tipo de implementación inyectable. Una enumeración en este caso es tan buena como cualquier otro tipo de configuración. Este es un ejemplo de una situación en la que no hay más oportunidades para reemplazar la "enumeración sucia" con polimorfismo. La ramificación por abstracción solo puede llevarte tan lejos.
Ant P
0

Si usar una enumeración es un olor a código o no, depende del contexto. Creo que puede obtener algunas ideas para responder a su pregunta si considera el problema de expresión . Entonces, tiene una colección de diferentes tipos y una colección de operaciones en ellos, y necesita organizar su código. Hay dos opciones simples:

  • Organice el código de acuerdo con las operaciones. En este caso, puede usar una enumeración para etiquetar diferentes tipos y tener una declaración de cambio en cada procedimiento que use los datos etiquetados.
  • Organice el código de acuerdo con los tipos de datos. En este caso, puede reemplazar la enumeración por una interfaz y utilizar una clase para cada elemento de la enumeración. Luego implementa cada operación como método en cada clase.

¿Qué solución es mejor?

Si, como señaló Karl Bielefeldt, sus tipos son fijos y espera que el sistema crezca principalmente al agregar nuevas operaciones en estos tipos, entonces usar una enumeración y tener una declaración de cambio es una mejor solución: cada vez que necesite una nueva operación, simplemente implemente un nuevo procedimiento, mientras que al usar clases tendría que agregar un método a cada clase.

Por otro lado, si espera tener un conjunto de operaciones bastante estable pero cree que tendrá que agregar más tipos de datos a lo largo del tiempo, usar una solución orientada a objetos es más conveniente: como se deben implementar nuevos tipos de datos, simplemente continúe agregando nuevas clases implementando la misma interfaz, mientras que si estuviera usando una enumeración, tendría que actualizar todas las instrucciones de cambio en todos los procedimientos que utilizan la enumeración.

Si no puede clasificar su problema en cualquiera de las dos opciones anteriores, puede buscar soluciones más sofisticadas (consulte, por ejemplo, nuevamente la página de Wikipedia citada anteriormente para una breve discusión y alguna referencia para leer más).

Por lo tanto, debe intentar comprender en qué dirección puede evolucionar su aplicación y luego elegir una solución adecuada.

Dado que los libros a los que se refiere tratan el paradigma orientado a objetos, no es sorprendente que estén predispuestos a usar enumeraciones. Sin embargo, una solución de orientación a objetos no siempre es la mejor opción.

En pocas palabras: las enumeraciones no son necesariamente un olor a código.

Giorgio
fuente
tenga en cuenta que la pregunta se hizo en el contexto de C #, un lenguaje OOP. también, que agrupar por operación o por tipo son dos caras de la misma moneda es interesante, pero no veo que una sea "mejor" que la otra como usted describe. la cantidad de código resultante es la misma, y ​​los lenguajes OOP ya facilitan la organización por tipo. También produce un código sustancialmente más intuitivo (fácil de leer).
Dave Cousineau
@DaveCousineau: "No veo que uno sea" mejor "que el otro como usted describe": No dije que uno siempre es mejor que el otro. Dije que uno es mejor que el otro dependiendo del contexto. Por ejemplo, si las operaciones son más o menos fijo y planea en la adición de nuevos tipos (por ejemplo, una interfaz gráfica de usuario con el fijo paint(), show(), close() resize()operaciones y widgets personalizados), entonces el enfoque orientado a objetos es mejor en el sentido de que permite añadir un nuevo tipo sin afectar demasiado mucho código existente (básicamente implementa una nueva clase, que es un cambio local).
Giorgio
Tampoco veo que sea "mejor" como usted describió, lo que significa que no veo que dependa de la situación. La cantidad de código que tiene que escribir es exactamente la misma en ambos escenarios. La única diferencia es que una forma es antitética a OOP (enumeraciones) y la otra no.
Dave Cousineau
@DaveCousineau: la cantidad de código que tiene que escribir es probablemente la misma, la localidad (lugares en el código fuente que debe cambiar y recompilar) cambia drásticamente. Por ejemplo, en OOP, si agrega un nuevo método a una interfaz, debe agregar una implementación para cada clase que implemente esa interfaz.
Giorgio
¿No es solo una cuestión de amplitud VS profundidad? Agregar 1 método a 10 archivos o 10 métodos a 1 archivo.
Dave Cousineau