¿SÓLIDO, evitando dominios anémicos, inyección de dependencia?

33

Aunque esta podría ser una pregunta independiente del lenguaje de programación, estoy interesado en respuestas dirigidas al ecosistema .NET.

Este es el escenario: supongamos que necesitamos desarrollar una aplicación de consola simple para la administración pública. La aplicación es sobre el impuesto de vehículos. Ellos (solo) tienen las siguientes reglas comerciales:

1.a) Si el vehículo es un automóvil y la última vez que su propietario pagó el impuesto fue hace 30 días, entonces el propietario debe pagar nuevamente.

1.b) Si el vehículo es una motocicleta y la última vez que su propietario pagó el impuesto fue hace 60 días, entonces el propietario tiene que pagar nuevamente.

En otras palabras, si tiene un automóvil, debe pagar cada 30 días o si tiene una motocicleta, debe pagar cada 60 días.

Para cada vehículo en el sistema, la aplicación debe probar esas reglas e imprimir aquellos vehículos (número de placa e información del propietario) que no los satisfacen.

Lo que quiero es:

2.a) Conforme a los principios SÓLIDOS (especialmente el principio Abierto / cerrado).

Lo que no quiero es (creo):

2.b) Un dominio anémico, por lo tanto, la lógica empresarial debe ir dentro de las entidades comerciales.

Empecé con esto:

public class Person
// You wanted a banana but what you got was a gorilla holding the banana and the entire jungle.
{
    public string Name { get; set; }

    public string Surname { get; set; }
}

public abstract class Vehicle
{
    public string PlateNumber { get; set; }

    public Person Owner { get; set; }

    public DateTime LastPaidTime { get; set; }

    public abstract bool HasToPay();
}

public class Car : Vehicle
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 30;
    }
}

public class Motorbike : Vehicle
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 60;
    }
}

public class PublicAdministration
{
    public IEnumerable<Vehicle> GetVehiclesThatHaveToPay()
    {
        return this.GetAllVehicles().Where(vehicle => vehicle.HasToPay());
    }

    private IEnumerable<Vehicle> GetAllVehicles()
    {
        throw new NotImplementedException();
    }
}

class Program
{
    static void Main(string[] args)
    {
        PublicAdministration administration = new PublicAdministration();
        foreach (var vehicle in administration.GetVehiclesThatHaveToPay())
        {
            Console.WriteLine("Plate number: {0}\tOwner: {1}, {2}", vehicle.PlateNumber, vehicle.Owner.Surname, vehicle.Owner.Name);
        }
    }
}

2.a: El principio abierto / cerrado está garantizado; si quieren el impuesto sobre la bicicleta que usted acaba de heredar del vehículo, entonces anula el método HasToPay y listo. Principio abierto / cerrado satisfecho por simple herencia.

Los "problemas" son:

3.a) ¿Por qué un vehículo tiene que saber si tiene que pagar? ¿No es una preocupación de la Administración Pública? Si la administración pública rige el cambio del impuesto sobre el automóvil, ¿por qué el automóvil tiene que cambiar? Creo que debería preguntarse: "¿por qué pones un método HasToPay dentro del vehículo?", Y la respuesta es: porque no quiero probar el tipo de vehículo (tipo de) dentro de PublicAdministration. ¿Ves una mejor alternativa?

3.b) Tal vez el pago de impuestos es una preocupación del vehículo después de todo, o tal vez mi diseño inicial de Administración de Persona-Vehículo-Automóvil-Moto-Público es simplemente incorrecto, o necesito un filósofo y un mejor analista de negocios. Una solución podría ser: mover el método HasToPay a otra clase, podemos llamarlo TaxPayment. Luego creamos dos clases derivadas de TaxPayment; CarTaxPayment y MotoTaxPayment. Luego agregamos una propiedad de pago abstracta (del tipo TaxPayment) a la clase Vehicle y devolvemos la instancia correcta de TaxPayment de las clases Car y Motorbike:

public abstract class TaxPayment
{
    public abstract bool HasToPay();
}

public class CarTaxPayment : TaxPayment
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 30;
    }
}

public class MotorbikeTaxPayment : TaxPayment
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 60;
    }
}

public abstract class Vehicle
{
    public string PlateNumber { get; set; }

    public Person Owner { get; set; }

    public DateTime LastPaidTime { get; set; }

    public abstract TaxPayment Payment { get; }
}

public class Car : Vehicle
{
    private CarTaxPayment payment = new CarTaxPayment();
    public override TaxPayment Payment
    {
        get { return this.payment; }
    }
}

public class Motorbike : Vehicle
{
    private MotorbikeTaxPayment payment = new MotorbikeTaxPayment();
    public override TaxPayment Payment
    {
        get { return this.payment; }
    }
}

E invocamos el viejo proceso de esta manera:

    public IEnumerable<Vehicle> GetVehiclesThatHaveToPay()
    {
        return this.GetAllVehicles().Where(vehicle => vehicle.Payment.HasToPay());
    }

Pero ahora este código no se compilará porque no hay un miembro de LastPaidTime dentro de CarTaxPayment / MotorbikeTaxPayment / TaxPayment. Estoy empezando a ver CarTaxPayment y MotorbikeTaxPayment más como "implementaciones de algoritmos" que como entidades comerciales. De alguna manera ahora esos "algoritmos" necesitan el valor LastPaidTime. Claro, podemos pasar el valor al constructor de TaxPayment, pero eso violaría la encapsulación del vehículo, o las responsabilidades, o como lo llamen esos evangelistas de OOP, ¿no?

3.c) Supongamos que ya tenemos un ObjectContext de Entity Framework (que usa nuestros objetos de dominio; Persona, Vehículo, Coche, Moto, Administración Pública). ¿Cómo haría para obtener una referencia ObjectContext del método PublicAdministration.GetAllVehicles y, por lo tanto, implementar la funcionalidad?

3.d) Si también necesitamos la misma referencia de ObjectContext para CarTaxPayment.HasToPay pero diferente para MotorbikeTaxPayment.HasToPay, ¿quién está a cargo de la "inyección" o cómo pasaría esas referencias a las clases de pago? En este caso, evitando un dominio anémico (y, por lo tanto, sin objetos de servicio), ¿no nos lleva al desastre?

¿Cuáles son sus opciones de diseño para este escenario simple? Claramente, los ejemplos que he mostrado son "demasiado complejos" para una tarea fácil, pero al mismo tiempo la idea es adherirse a los principios SÓLIDOS y prevenir dominios anémicos (de los cuales se ha encargado destruir).

David Robert Jones
fuente

Respuestas:

15

Yo diría que ni una persona ni un vehículo deben saber si se debe un pago.

reexaminar el dominio del problema

Si observa el dominio del problema "personas que tienen automóviles": para comprar un automóvil, tiene algún tipo de contrato que transfiere la propiedad de una persona a otra, ni el automóvil ni la persona cambian en ese proceso. Tu modelo carece por completo de eso.

experimento mental

Suponiendo que hay una pareja casada, el hombre es dueño del automóvil y está pagando los impuestos. Ahora el hombre muere, su esposa hereda el automóvil y pagará los impuestos. Sin embargo, sus platos permanecerán igual (al menos en mi país lo harán). ¡Sigue siendo el mismo auto! Debería poder modelar eso en su dominio.

=> Propiedad

Un automóvil que no pertenece a nadie sigue siendo un automóvil, pero nadie pagará impuestos por él. De hecho, no está pagando impuestos por el automóvil, sino por el privilegio de ser propietario de un automóvil . Entonces mi propuesta sería:

public class Person
{
    public string Name { get; set; }

    public string Surname { get; set; }

    public List<Ownership> getOwnerships();

    public List<Vehicle> getVehiclesOwned();
}

public abstract class Vehicle
{
    public string PlateNumber { get; set; }

    public Ownership currentOwnership { get; set; }

}

public class Car : Vehicle {}

public class Motorbike : Vehicle {}

public abstract class Ownership
{
    public Ownership(Vehicle vehicle, Owner owner);

    public Vehicle Vehicle { get;}

    public Owner CurrentOwner { get; set; }

    public abstract bool HasToPay();

    public DateTime LastPaidTime { get; set; }

   //Could model things like payment history or owner history here as well
}

public class CarOwnership : Ownership
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 30;
    }
}

public class MotorbikeOwnership : Ownership
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 60;
    }
}

public class PublicAdministration
{
    public IEnumerable<Ownership> GetVehiclesThatHaveToPay()
    {
        return this.GetAllOwnerships().Where(HasToPay());
    }

}

Esto está lejos de ser perfecto y probablemente ni siquiera remotamente corrija el código C #, pero espero que entiendas la idea (y alguien podría limpiar esto). El único inconveniente es que probablemente tendría una fábrica o algo para crear la correcta CarOwnershipentre una Cary unaPerson

sebastiangeiger
fuente
Creo que el vehículo en sí debería registrar los impuestos pagados por él, y las personas deberían registrar los impuestos pagados por todos sus vehículos. Se podría escribir un código de impuestos para que si el impuesto sobre un automóvil se paga el 1 de junio y se vende el 10 de junio, el nuevo propietario podría ser responsable del impuesto el 10 de junio, el 1 de julio o el 10 de julio (e incluso si es actualmente una forma podría cambiar). Si la regla de los 30 días es constante para el automóvil incluso a través de cambios de propiedad, pero los automóviles que nadie le debe no pueden pagar impuestos, entonces sugeriría que cuando se compre un automóvil que no haya sido propietario durante 30 días o más, pago de impuestos ...
supercat
... obtener el registro de una persona ficticia de "crédito fiscal para vehículos no propietarios", de modo que, al momento de la venta, la responsabilidad tributaria sea cero o treinta días, independientemente de cuánto tiempo haya estado sin dueño el vehículo.
supercat
4

Creo que en este tipo de situación, donde desea que las reglas de negocio existan fuera de los objetos de destino, las pruebas de tipo son más que aceptables.

Ciertamente, en muchas situaciones, debe usar un patrón de estrategia y dejar que los objetos manejen las cosas por sí mismos, pero eso no siempre es deseable.

En el mundo real, un empleado miraría el tipo de vehículo y buscaría sus datos fiscales basándose en eso. ¿Por qué su software no debería seguir la misma regla de busienss?

Erik Funkenbusch
fuente
Ok, digamos que me convenciste y desde el método GetVehiclesThatHaveToPay inspeccionamos el tipo de vehículo. ¿Quién debería ser responsable del LastPaidTime? ¿LastPaidTime es una preocupación del Vehículo o de una Administración Pública? Porque si se trata de un vehículo, ¿no debería residir esa lógica empresarial en el vehículo? ¿Qué me puede decir sobre la pregunta 3.c?
@DavidRobertJones - Idealmente, buscaría el último pago en el registro del historial de pagos, basado en la licencia del vehículo. Un pago de impuestos es una preocupación fiscal, no una preocupación de vehículo.
Erik Funkenbusch 01 de
55
@DavidRobertJones Creo que te estás confundiendo con tu propia metáfora. Lo que tienes no es un vehículo real que necesita hacer todas las cosas que hace un vehículo. En cambio, ha nombrado, por simplicidad, un Vehículo sujeto a impuestos (o algo similar) a un Vehículo. Todo su dominio es el pago de impuestos. No te preocupes por lo que hacen los vehículos reales. Y, si es necesario, suelte la metáfora y proponga una más apropiada.
3

Lo que no quiero es (creo):

2.b) Un dominio anémico, que significa lógica empresarial dentro de las entidades comerciales.

Tienes esto al revés. Un dominio anémico es aquel cuya lógica está fuera de las entidades comerciales. Hay muchas razones por las que usar clases adicionales para implementar la lógica es una mala idea; y solo un par de por qué deberías hacerlo.

De ahí la razón por la que se llama anémica: la clase no tiene nada.

Que haría yo:

  1. Cambia tu clase de vehículo abstracta para que sea una interfaz.
  2. Agregue una interfaz ITaxPayment que los vehículos deben implementar. Haga que su HasToPay cuelgue de aquí.
  3. Elimine las clases MotorbikeTaxPayment, CarTaxPayment, TaxPayment. Son complicaciones innecesarias / desorden.
  4. Cada tipo de vehículo (automóvil, bicicleta, autocaravana) debe implementarse como mínimo Vehículo y, si están sujetos a impuestos, también debe implementar ITaxPayment. De esta manera, puede delinear entre los vehículos que no están sujetos a impuestos y los que sí ... Suponiendo que esta situación incluso exista.
  5. Cada tipo de vehículo debe implementar, dentro de los límites de la clase misma, los métodos definidos en sus interfaces.
  6. Además, agregue la última fecha de pago a su interfaz ITaxPayment.
Yo no
fuente
Tienes razón. Originalmente la pregunta no se formuló de esa manera, eliminé una parte y el significado cambió. Ya está arreglado. ¡Gracias!
2

¿Por qué un vehículo tiene que saber si tiene que pagar? ¿No es una preocupación de la Administración Pública?

No. Según tengo entendido, toda su aplicación se trata de impuestos. Por lo tanto, la preocupación acerca de si un vehículo debe pagar impuestos no es más una preocupación de la Administración Pública que cualquier otra cosa en todo el programa. No hay razón para ponerlo en otro lugar que no sea aquí.

public class Car : Vehicle
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 30;
    }
}

public class Motorbike : Vehicle
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 60;
    }
}

Estos dos fragmentos de código difieren solo por la constante. En lugar de anular toda la función HasToPay (), anule una int DaysBetweenPayments()función. Llame a eso en lugar de usar la constante. Si esto es todo en lo que realmente difieren, abandonaría las clases.

También sugeriría que Moto / Coche debería ser una propiedad de categoría de vehículo en lugar de subclases. Eso te da más flexibilidad. Por ejemplo, hace que sea fácil cambiar la categoría de una motocicleta o automóvil. Por supuesto, es poco probable que las bicicletas se transformen en automóviles en la vida real. Pero usted sabe que un vehículo va a ingresar mal y debe cambiarse más tarde.

Claro, podemos pasar el valor al constructor de TaxPayment, pero eso violaría la encapsulación del vehículo, o las responsabilidades, o como lo llamen esos evangelistas de OOP, ¿no?

Realmente no. Un objeto que pasa deliberadamente sus datos a otro objeto como parámetro no es una gran preocupación de encapsulación. La verdadera preocupación es que otros objetos lleguen dentro de este objeto para extraer datos o manipularlos dentro del objeto.

Winston Ewert
fuente
1

Puede que estés en el camino correcto. El problema es, como se habrá dado cuenta, que no hay un miembro de LastPaidTime dentro de TaxPayment . Ahí es exactamente a donde pertenece, aunque no necesita exponerse públicamente en absoluto:

public interface ITaxPayment
{
    // Your base TaxPayment class will know the 
    // next due date. But you can easily create
    // one which uses (for example) fixed dates
    bool IsPaymentDue();
}

public interface IVehicle
{
    string Plate { get; }
    Person Owner { get; }
    ITaxPayment LastTaxPayment { get; }
} 

Cada vez que recibe un pago, crea una nueva instancia de ITaxPaymentacuerdo con las políticas actuales, que puede tener reglas completamente diferentes a la anterior.

Groo
fuente
La cuestión es que el pago adeudado depende de la última vez que el propietario pagó (diferentes vehículos, diferentes fechas de vencimiento). ¿Cómo sabe ITaxPayment cuál fue la última vez que un vehículo (o propietario) pagó para hacer el cálculo?
1

Estoy de acuerdo con la respuesta de @sebastiangeiger de que el problema es realmente sobre la propiedad de vehículos en lugar de los vehículos. Voy a sugerirle que use su respuesta pero con algunos cambios. Haría que la Ownershipclase sea una clase genérica:

public class Vehicle {}

public abstract class Ownership<T>
{
    public T Item { get; set; }

    public DateTime LastPaidTime { get; set; }

    public abstract TimeSpan PaymentInterval { get; }

    public virtual bool HasToPay
    {
        get { return (DateTime.Today - this.LastPaidTime) >= this.PaymentInterval; }
    }
}

Y use la herencia para especificar el intervalo de pago para cada tipo de vehículo. También sugiero usar la TimeSpanclase fuertemente tipada en lugar de enteros simples:

public class CarOwnership : Ownership<Vehicle>
{
    public override TimeSpan PaymentInterval
    {
        get { return new TimeSpan(30, 0, 0, 0); }
    }
}

public class MotorbikeOwnership : Ownership<Vehicle>
{
    public override TimeSpan PaymentInterval
    {
        get { return new TimeSpan(60, 0, 0, 0); }
    }
}

Ahora su código real solo necesita tratar con una clase - Ownership<Vehicle>.

public class PublicAdministration
{
    List<Ownership<Vehicle>> VehicleOwnerships { get; set; }

    public IEnumerable<Vehicle> GetVehiclesThatHaveToPay()
    {
        return VehicleOwnerships.Where(x => x.HasToPay).Select(x => x.Item);
    }
}
usuario74130
fuente
0

Odio decírtelo, pero tienes un dominio anémico , es decir, lógica de negocios fuera de los objetos. Si esto es lo que está buscando, está bien, pero debe leer sobre las razones para evitarlo.

Intente no modelar el dominio del problema, pero modele la solución. Es por eso que está terminando con jerarquías de herencia paralelas y más complejidad de la que necesita.

Algo como esto es todo lo que necesita para ese ejemplo:

public class Vehicle
{
    public Boolean isMotorBike()
    public string PlateNumber { get; set; }
    public String Owner { get; set; }
    public DateTime LastPaidTime { get; set; }
}

public class TaxPaymentCalculator
{
    public List<Vehicle> GetVehiclesThatHaveToPay(List<Vehicle> all)
}

Si quieres seguir a SOLID, eso es genial, pero como dijo el tío Bob, son solo principios de ingeniería . No están destinados a ser aplicados estrictamente, pero son pautas que deben considerarse.

Garrett Hall
fuente
44
Creo que su solución no es particularmente escalable. Debería agregar un nuevo booleano para cada tipo de vehículo que agregue. Esa es una mala elección en mi opinión.
Erik Funkenbusch 01 de
@Garrett Hall, me gustó que hayas eliminado la clase Persona de mi "dominio". No tendría esa clase en primer lugar, lo siento mucho por eso. Pero isMotorBike no tiene sentido. ¿Cuál sería la implementación?
1
@DavidRobertJones: Estoy de acuerdo con Mystere, agregar un isMotorBikemétodo no es una buena opción de diseño de OOP. Es como reemplazar el polimorfismo con un código de tipo (aunque sea booleano).
Groo
@MystereMan, uno podría soltar fácilmente los bools y usar unenum Vehicles
radarbob
+1. Intente no modelar el dominio del problema, pero modele la solución . Concisa . Memorablemente dicho. Y los principios comentan. Veo demasiados intentos de interpretación literal estricta ; disparates.
radarbob
0

Creo que tiene razón en que el período de pago no es propiedad del automóvil / moto real. Pero es un atributo de la clase de vehículo.

Si bien podría seguir el camino de tener un if / select en el Tipo de objeto, esto no es muy escalable. Si luego agrega un camión y un Segway y empuja la bicicleta, el autobús y el avión (puede parecer poco probable, pero nunca se sabe con los servicios públicos), la condición se agrava. Y si desea cambiar las reglas de un camión, debe encontrarlo en esa lista.

Entonces, ¿por qué no convertirlo, literalmente, en un atributo y usar la reflexión para sacarlo? Algo como esto:

[DaysBetweenPayments(30)]
public class Car : Vehicle
{
    // actual properties of the physical vehicle
}

[AttributeUsage(AttributeTargets.Class, Inherited = false)]
public class DaysBetweenPaymentsAttribute : Attribute, IPaymentRequiredCalculator
{
    private int _days;

    public DaysBetweenPaymentAttribute(int days)
    {
        _days = days;
    }

    public bool HasToPay(Vehicle vehicle)
    {
        return vehicle.LastPaid.AddDays(_days) <= DateTime.Now;
    }
}

También podría ir con una variable estática, si eso es más cómodo.

Los inconvenientes son

  • será un poco más lento y supongo que tienes que procesar muchos vehículos. Si eso es un problema, entonces podría tener una visión más pragmática y seguir con la propiedad abstracta o el enfoque interconectado. (Aunque, también consideraría el almacenamiento en caché por tipo).

  • Tendrá que validar en el inicio que cada subclase de Vehículo tiene un atributo IPaymentRequiredCalculator (o permitir un valor predeterminado), porque no desea que procese 1000 autos, solo para colisionar porque Moto no tiene un PaymentPeriod.

La ventaja es que si luego se encuentra con un mes calendario o un pago anual, puede escribir una nueva clase de atributo. Esta es la definición misma de OCP.

pdr
fuente
El principal problema con esto es que si desea cambiar el número de días entre pagos para un Vehículo, deberá reconstruir y volver a implementar, y si la frecuencia de pago varía según una propiedad de la entidad (por ejemplo, el tamaño del motor) será difícil de conseguir una solución elegante para eso.
Ed James
@EdWoodcock: Creo que el primer problema es cierto para la mayoría de las soluciones y realmente no me preocuparía. Si quieren ese nivel de flexibilidad más tarde, les daría una aplicación DB. Sobre el segundo punto, estoy completamente en desacuerdo. En ese momento, solo agrega vehículo como argumento opcional a HasToPay () e ignóralo donde no lo necesites.
pdr
Supongo que depende de los planes a largo plazo que tenga para la aplicación, además de los requisitos particulares del cliente, en cuanto a si vale la pena conectar un DB si ya no existe ninguno (tiendo a suponer que casi todo el software es DB -conducido, que sé que no siempre es el caso). Sin embargo, en el segundo punto, el calificador importante es elegante ; No diría que agregar un parámetro adicional a un método en un atributo que luego toma una instancia de la clase a la que está asociado el atributo es una solución particularmente elegante. Pero, de nuevo, depende de los requisitos de su cliente.
Ed James
@EdWoodcock: En realidad, solo estaba pensando en esto y el argumento de LastPaid ya tendrá que ser una propiedad de Vehicle. Con su comentario en mente, podría valer la pena reemplazar ese argumento ahora, en lugar de más tarde, se editará.
pdr