¿El patrón de visitante es válido en este escenario?

9

El objetivo de mi tarea es diseñar un sistema pequeño que pueda ejecutar tareas recurrentes programadas. Una tarea recurrente es algo así como "enviar un correo electrónico al administrador cada hora de 8:00 a. M. A 5:00 p. M., De lunes a viernes".

Tengo una clase base llamada RecurringTask .

public abstract class RecurringTask{

    // I've already figured out this part
    public bool isOccuring(DateTime dateTime){
        // implementation
    }

    // run the task
    public abstract void Run(){

    }
}

Y tengo varias clases que se heredan de RecurringTask . Uno de ellos se llama SendEmailTask .

public class SendEmailTask : RecurringTask{
    private Email email;

    public SendEmailTask(Email email){
        this.email = email;
    }

    public override void Run(){
        // need to send out email
    }
}

Y tengo un servicio de correo electrónico que me puede ayudar a enviar un correo electrónico.

La última clase es RecurringTaskScheduler , es responsable de cargar tareas desde la caché o la base de datos y ejecutar la tarea.

public class RecurringTaskScheduler{

    public void RunTasks(){
        // Every minute, load all tasks from cache or database
        foreach(RecuringTask task : tasks){
            if(task.isOccuring(Datetime.UtcNow)){
                task.run();
            }
        }
    }
}

Aquí está mi problema: ¿dónde debo poner EmailService ?

Opción 1 : inyectar EmailService en SendEmailTask

public class SendEmailTask : RecurringTask{
    private Email email;

    public EmailService EmailService{ get; set;}

    public SendEmailTask (Email email, EmailService emailService){
        this.email = email;
        this.EmailService = emailService;
    }

    public override void Run(){
        this.EmailService.send(this.email);
    }
}

Ya hay algunas discusiones sobre si debemos inyectar un servicio en una entidad, y la mayoría de la gente está de acuerdo en que no es una buena práctica. Ver este artículo .

Opción 2: si ... lo demás en RecurringTaskScheduler

public class RecurringTaskScheduler{
    public EmailService EmailService{get;set;}

    public class RecurringTaskScheduler(EmailService emailService){
        this.EmailService = emailService;
    }

    public void RunTasks(){
        // load all tasks from cache or database
        foreach(RecuringTask task : tasks){
            if(task.isOccuring(Datetime.UtcNow)){
                if(task is SendEmailTask){
                    EmailService.send(task.email); // also need to make email public in SendEmailTask
                }
            }
        }
    }
}

Me han dicho si ... De lo contrario, el elenco como el anterior no es OO, y traerá más problemas.

Opción 3: cambie la firma de Ejecutar y cree ServiceBundle .

public class ServiceBundle{
    public EmailService EmailService{get;set}
    public CleanDiskService CleanDiskService{get;set;}
    // and other services for other recurring tasks

}

Inyecte esta clase en RecurringTaskScheduler

public class RecurringTaskScheduler{
    public ServiceBundle ServiceBundle{get;set;}

    public class RecurringTaskScheduler(ServiceBundle serviceBundle){
        this.ServiceBundle = ServiceBundle;
    }

    public void RunTasks(){
        // load all tasks from cache or database
        foreach(RecuringTask task : tasks){
            if(task.isOccuring(Datetime.UtcNow)){
                task.run(serviceBundle);
            }
        }
    }
}

El método Run de SendEmailTask sería

public void Run(ServiceBundle serviceBundle){
    serviceBundle.EmailService.send(this.email);
}

No veo grandes problemas con este enfoque.

Opción 4 : Patrón de visitante.
La idea básica es crear un visitante que encapsule servicios al igual que ServiceBundle .

public class RunTaskVisitor : RecurringTaskVisitor{
    public EmailService EmailService{get;set;}
    public CleanDiskService CleanDiskService{get;set;}

    public void Visit(SendEmailTask task){
        EmailService.send(task.email);
    }

    public void Visit(ClearDiskTask task){
        //
    }
}

Y también necesitamos cambiar la firma del método Run . El método Run de SendEmailTask es

public void Run(RecurringTaskVisitor visitor){
    visitor.visit(this);
}

Es una implementación típica del Patrón de visitante, y el visitante será inyectado en RecurringTaskScheduler .

En resumen: entre estos cuatro enfoques, ¿cuál es el mejor para mi escenario? ¿Y hay alguna gran diferencia entre la Opción 3 y la Opción 4 para este problema?

¿O tienes una mejor idea sobre este problema? ¡Gracias!

Actualización 22/05/2015 : Creo que la respuesta de Andy resume muy bien mi intención; Si todavía está confundido sobre el problema en sí, sugiero leer su publicación primero.

Me acabo de enterar que mi problema es muy similar al problema de Despacho de mensajes , que lleva a la Opción 5.

Opción 5 : Convertir mi problema a Despacho de mensajes .
Hay un mapeo uno a uno entre mi problema y el problema de envío de mensajes :

Despachador de mensajes : reciba IMessage y envíe subclases de IMessage a sus controladores correspondientes. → RecurringTaskScheduler

IMessage : una interfaz o una clase abstracta. → Tarea recurrente

Mensaje A : se extiende desde IMessage y tiene información adicional. → SendEmailTask

MensajeB : Otra subclase de IMessage . → CleanDiskTask

MessageAHandler : cuando reciba MessageA , trátelo → SendEmailTaskHandler, que contiene EmailService, y enviará un correo electrónico cuando reciba SendEmailTask

MessageBHandler : igual que MessageAHandler , pero maneja MessageB en su lugar. → CleanDiskTaskHandler

La parte más difícil es cómo enviar diferentes tipos de IMessage a diferentes manejadores. Aquí hay un enlace útil .

Realmente me gusta este enfoque, no contamina mi entidad con el servicio, y no tiene ninguna clase de Dios .

Sher10ck
fuente
No ha etiquetado un idioma o plataforma, pero le recomiendo que busque en cron . Su plataforma puede tener una biblioteca que funciona de manera similar (por ejemplo, jcron que parece un poco difunto). Programar trabajos y tareas es en gran medida un problema resuelto: ¿ha buscado otras opciones antes de implementar las suyas? ¿Hubo razones para no usarlos?
@Snowman Podemos cambiar a una biblioteca madura más tarde. Todo depende de mi gerente. La razón por la que publico esta pregunta es porque quiero encontrar una manera de resolver este 'tipo' de problema. He visto este tipo de problema más de una vez, y no pude encontrar una solución elegante. Entonces me pregunto si hice algo mal.
Sher10ck
Sin embargo, siempre trato de recomendar la reutilización del código si es posible.
1
SendEmailTaskme parece más un servicio que una entidad. Iría por la opción 1 sin dudarlo.
Bart van Ingen Schenau
3
Lo que falta (para mí) para Visitor es la estructura de clase que accepts visitantes. La motivación para Visitor es que tiene muchos tipos de clases en algunos agregados que necesitan visitas, y no es conveniente modificar su código para cada nueva funcionalidad (operación). Todavía no veo cuáles son esos objetos agregados, y creo que Visitor no es apropiado. Si es el caso, debe editar su pregunta (que se refiere al visitante).
Fuhrmanator

Respuestas:

4

Yo diría que la opción 1 es la mejor ruta a seguir. La razón por la que no debe descartarlo es que noSendEmailTask es una entidad. Una entidad es un objeto relacionado con la retención de datos y estado. Tu clase tiene muy poco de eso. De hecho, no es una entidad, pero contiene una entidad: el objeto que está almacenando. Eso significa que no debe tomar un servicio, o tener un método. En cambio, debe tener servicios que tomen entidades, como su . Entonces ya está siguiendo la idea de mantener los servicios fuera de las entidades.EmailEmail#SendEmailService

Como SendEmailTaskno es una entidad, por lo tanto, está perfectamente bien inyectar el correo electrónico y el servicio, y eso debe hacerse a través del constructor. Al hacer la inyección del constructor, podemos estar seguros de que SendEmailTasksiempre está listo para realizar su trabajo.

Ahora veamos por qué no hacer las otras opciones (específicamente con respecto a SOLID ).

opcion 2

Te han dicho con razón que ramificarte en ese tipo traerá más dolores de cabeza en el futuro. Veamos por qué. Primero, ifs tienden a agruparse y crecer. Hoy es una tarea enviar correos electrónicos, mañana, cada tipo diferente de clase necesita un servicio diferente u otro comportamiento. Gestionar esa ifdeclaración se convierte en una pesadilla. Dado que estamos ramificando en tipo (y en este caso tipo explícito ), estamos subvirtiendo el sistema de tipos integrado en nuestro idioma.

La opción 2 no es la responsabilidad única (SRP) porque lo que antes era reutilizable RecurringTaskSchedulerahora tiene que saber sobre todos estos diferentes tipos de tareas y sobre los diferentes tipos de servicios y comportamientos que podrían necesitar. Esa clase es mucho más difícil de reutilizar. Tampoco es Abierto / Cerrado (OCP). Debido a que necesita saber sobre este tipo de tarea o aquella (o este tipo de servicio o ese), los cambios dispares en las tareas o servicios podrían forzar cambios aquí. Agregar una nueva tarea? Agregar un nuevo servicio? ¿Cambiar la forma en que se maneja el correo electrónico? Cambio RecurringTaskScheduler. Debido a que el tipo de tarea es importante, no se adhiere a la sustitución de Liskov (LSP). No puede simplemente obtener una tarea y hacerse. Tiene que pedir el tipo y, según el tipo, haga esto o aquello. En lugar de encapsular las diferencias en las tareas, estamos metiendo todo eso en el RecurringTaskScheduler.

Opción 3

La opción 3 tiene algunos grandes problemas. Incluso en el artículo al que se vincula , el autor desalienta hacer esto:

  • Todavía puede usar un localizador de servicio estático ...
  • Evito el localizador de servicios cuando puedo, especialmente cuando el localizador de servicios debe ser estático ...

Estás creando un localizador de servicios con tu ServiceBundleclase. En este caso, no parece ser estático, pero aún tiene muchos de los problemas inherentes a un localizador de servicios. Sus dependencias ahora están ocultas debajo de esto ServiceBundle. Si te doy la siguiente API de mi nueva y genial tarea:

class MyCoolNewTask implements RecurringTask
{
    public bool isOccuring(DateTime dateTime) {
        return true; // It's always happenin' here!
    }

    public void Run(ServiceBundle bundle) {
        // yeah, some awesome stuff here
    }
}

¿Cuáles son los servicios que estoy usando? ¿Qué servicios se deben burlar en una prueba? ¿Qué es lo que me impide usar todos los servicios del sistema, solo porque?

Si quiero usar su sistema de tareas para ejecutar algunas tareas, ahora dependo de todos los servicios de su sistema, incluso si solo uso algunos o ninguno.

El ServiceBundlerealmente no es SRP porque necesita saber acerca de cada servicio en su sistema. Tampoco es OCP. Agregar nuevos servicios significa cambios en el ServiceBundle, y cambios en el ServiceBundlepueden significar cambios dispares en las tareas en otros lugares. ServiceBundleno segrega su interfaz (ISP). Tiene una interfaz en expansión de todos estos servicios, y debido a que es solo un proveedor de esos servicios, podríamos considerar que su interfaz abarca las interfaces de todos los servicios que proporciona también. Las tareas ya no se adhieren a la Inversión de dependencia (DIP), porque sus dependencias están ocultas detrás de ServiceBundle. Esto tampoco se adhiere al Principio de Menos Conocimiento (también conocido como la Ley de Deméter) porque las cosas saben muchas más cosas de las que tienen que saber.

Opcion 4

Anteriormente, tenía muchos objetos pequeños que podían funcionar de forma independiente. La opción 4 toma todos estos objetos y los junta en un solo Visitorobjeto. Este objeto actúa como un objeto de dios sobre todas sus tareas. Reduce sus RecurringTaskobjetos a sombras anémicas que simplemente llaman a un visitante. Todo el comportamiento se mueve al Visitor. ¿Necesitas cambiar el comportamiento? ¿Necesita agregar una nueva tarea? Cambio Visitor.

La parte más desafiante es que, debido a que todos los diferentes comportamientos están en una sola clase, cambiar algunos polimórficos arrastra a todos los demás comportamientos. Por ejemplo, queremos tener dos formas diferentes de enviar correos electrónicos (¿tal vez deberían usar servidores diferentes?). ¿Como podríamos hacerlo? Podríamos crear una IVisitorinterfaz e implementar ese código potencialmente duplicado, como el #Visit(ClearDiskTask)de nuestro visitante original. Luego, si encontramos una nueva forma de borrar un disco, tenemos que implementar y duplicar nuevamente. Entonces queremos ambos tipos de cambios. Implementar y duplicar nuevamente. Estos dos comportamientos diferentes y dispares están inextricablemente vinculados.

¿Tal vez podríamos subclasificar Visitor? Subclase con nuevo comportamiento de correo electrónico, subclase con nuevo comportamiento de disco. ¡Sin duplicación hasta ahora! ¿Subclase con ambos? Ahora uno u otro necesita ser duplicado (o ambos si así lo prefiere).

Comparemos con la opción 1: necesitamos un nuevo comportamiento de correo electrónico. Podemos crear un nuevo RecurringTaskque haga el nuevo comportamiento, inyectar en sus dependencias y agregarlo a la colección de tareas en el RecurringTaskScheduler. Ni siquiera necesitamos hablar sobre la limpieza de discos, porque esa responsabilidad está en otro lugar por completo. También tenemos toda la gama de herramientas OO a nuestra disposición. Podríamos decorar esa tarea con el registro, por ejemplo.

La opción 1 le causará menos dolor y es la forma más correcta de manejar esta situación.

cbojar
fuente
¡Su análisis en Otion2,3,4 es fantástico! Realmente me ayuda mucho. Pero para la Opción 1, diría que * SendEmailTask ​​* es una entidad. Tiene id, tiene su patrón recurrente y otra información útil que debe almacenarse en db. Creo que Andy resume bien mi intención. Quizás un nombre como * EMailTaskDefinitions * sea más apropiado. No quiero contaminar mi entidad con mi código de servicio. Euphoric menciona algún problema si inyecto un servicio en una entidad. También actualizo mi pregunta e incluyo la Opción 5, que creo que es la mejor solución hasta ahora.
Sher10ck
@ Sher10ck Si está extrayendo la configuración de su SendEmailTaskbase de datos, entonces esa configuración debería ser una clase de configuración separada que también debería inyectarse en su SendEmailTask. Si está generando datos desde su SendEmailTask, debe crear un objeto de recuerdo para almacenar el estado y ponerlo en su base de datos.
cbojar
Necesito extraer la configuración de db, entonces, ¿estás sugiriendo inyectar ambos EMailTaskDefinitionsy EmailServiceen SendEmailTask? Luego, en el RecurringTaskScheduler, necesito inyectar algo como SendEmailTaskRepositorycuya responsabilidad es cargar la definición y el servicio e inyectarlos SendEmailTask. Pero ahora diría que es RecurringTaskSchedulernecesario conocer el Repositorio de cada tarea, como CleanDiskTaskRepository. Y necesito cambiar RecurringTaskSchedulercada vez que tengo una nueva tarea (para agregar el repositorio en el Programador).
Sher10ck
@ Sher10ck The RecurringTaskSchedulersolo debe tener en cuenta el concepto de un repositorio de tareas generalizado y a RecurringTask. Al hacer esto, puede depender de abstracciones. Los repositorios de tareas se pueden inyectar en el constructor de RecurringTaskScheduler. Entonces, los diferentes repositorios solo necesitan saber dónde RecurringTaskSchedulerse instancia (o podrían ocultarse en una fábrica y llamarse desde allí). Debido a que solo depende de las abstracciones, RecurringTaskSchedulerno necesita cambiar con cada nueva tarea. Esa es la esencia de la inversión de dependencia.
cbojar
3

¿Ha echado un vistazo a las bibliotecas existentes, por ejemplo, cuarzo de primavera o lote de primavera (no estoy seguro de qué se adapta mejor a sus necesidades)?

A su pregunta:

Supongo que el problema es que desea conservar algunos metadatos de la tarea de forma polimórfica, por lo que una tarea de correo electrónico tiene direcciones de correo electrónico asignadas, una tarea de registro a nivel de registro, etc. Puede almacenar una lista de aquellos en la memoria o en su base de datos, pero para separar las preocupaciones, no desea que la entidad esté contaminada con el código de servicio.

Mi solución propuesta:

Separaría la parte de ejecución y la parte de datos de la tarea, para tener, por ejemplo, TaskDefinitiony a TaskRunner. TaskDefinition tiene una referencia a un TaskRunner o una fábrica que crea uno (por ejemplo, si se requiere alguna configuración como smtp-host). La fábrica es específica: solo puede manejar EMailTaskDefinitionsy devuelve solo instancias de EMailTaskRunners. De esta forma, es más OO y seguro: si introduce un nuevo tipo de tarea, debe introducir una nueva fábrica específica (o reutilizar una), si no lo hace, no puede compilar.

De esta manera, terminaría con una dependencia: capa de entidad -> capa de servicio y viceversa, porque el Runner necesita información almacenada en la entidad y probablemente quiera actualizar su estado en la base de datos.

Puede romper el círculo utilizando una fábrica genérica, que toma una definición de tarea y devuelve un TaskRunner específico , pero eso requeriría muchos ifs. Usted podría utilizar la reflexión para encontrar un corredor que se nombra de manera similar como su definición, pero tenga cuidado este enfoque puede costar un poco de rendimiento, y puede dar lugar a errores de ejecución.

PD: estoy asumiendo Java aquí. Creo que es similar en .net. El principal problema aquí es el doble enlace.

Al patrón visitante

Creo que estaba destinado a ser utilizado para intercambiar un algoritmo para diferentes tipos de objetos de datos en tiempo de ejecución, que para fines de doble enlace puro. Por ejemplo, si tiene diferentes tipos de seguros y diferentes tipos de cálculo, por ejemplo, porque diferentes países lo requieren. Luego, elige un método de cálculo específico y lo aplica a varios seguros.

En su caso, elegiría una estrategia de tarea específica (por ejemplo, correo electrónico) y la aplicaría a todas sus tareas, lo cual está mal porque no todas son tareas de correo electrónico.

PD: No lo probé, pero creo que su Opción 4 tampoco funcionará, porque es de doble enlace nuevamente.

Andy
fuente
Resume muy bien mi intención, gracias! Me gustaría romper el círculo. Porque dejar que TaskDefiniton contenga una referencia a TaskRunner o factory tiene el mismo problema que Option1. Trato a la fábrica o TaskRunner como servicio. Si TaskDefinition necesita contiene una referencia a ellos, debe inyectar el servicio en TaskDefinition o utilizar algún método estático, que estoy tratando de evitar.
Sher10ck
1

Estoy completamente en desacuerdo con ese artículo. Los servicios (concretamente su "API") son parte importante del dominio comercial y, como tal, existirán dentro del modelo de dominio. Y no hay ningún problema con las entidades en el dominio empresarial que hacen referencia a otra cosa en el mismo dominio empresarial.

Cuando X envía un correo a Y.

Es una regla de negocios. Y para hacer eso, se necesita un servicio que envíe correo. Y la entidad que maneja When Xdebe saber sobre este servicio.

Pero hay algunos problemas con la implementación. Debe ser transparente para el usuario de la entidad, que la entidad está utilizando un servicio. Por lo tanto, agregar el servicio en constructor no es algo bueno. Esto también es un problema cuando deserializa la entidad de la base de datos, porque necesita establecer tanto los datos de la entidad como las instancias de los servicios. La mejor solución que se me ocurre es utilizar la inyección de propiedades después de crear la entidad. Tal vez forzando a cada instancia recién creada de cualquier entidad a pasar por el método "inicializar" que inyecta todas las entidades que la entidad necesita.

Eufórico
fuente
¿Con qué artículo te refieres que no estás de acuerdo? Sin embargo, un punto de vista interesante sobre el modelo de dominio. Probablemente pueda verlo así, sin embargo, las personas generalmente evitan mezclar servicios en entidades, ya que creará un acoplamiento estrecho muy pronto.
Andy
@Andy La que Sher10ck hizo referencia en su pregunta. Y no veo cómo crearía un acoplamiento apretado. Cualquier código mal escrito puede introducir un acoplamiento estrecho.
Eufórico
1

Esa es una gran pregunta y un problema interesante. Le propongo que use una combinación de cadena de responsabilidad y patrones de doble distribución (ejemplos de patrones aquí ).

Primero definamos la jerarquía de tareas. Tenga en cuenta que ahora hay varios runmétodos para implementar Double Dispatch.

public abstract class RecurringTask {

    public abstract boolean isOccuring(Date date);

    public boolean run(EmailService emailService) {
        return false;
    }

    public boolean run(ExecuteService executeService) {
        return false;
    }
}

public class SendEmailTask extends RecurringTask {

    private String email;

    public SendEmailTask(String email) {
        this.email = email;
    }

    @Override
    public boolean isOccuring(Date date) {
        return true;
    }

    @Override
    public boolean run(EmailService emailService) {
        emailService.runTask(this);
        return true;
    }

    public String getEmail() {
        return email;
    }
}

public class ExecuteTask extends RecurringTask {

    private String program;

    public ExecuteTask(String program) {
        this.program = program;
    }

    @Override
    public boolean isOccuring(Date date) {
        return true;
    }

    public String getName() {
        return program;
    }

    @Override
    public boolean run(ExecuteService executeService) {
        executeService.runTask(this);
        return true;
    }
}

A continuación, definamos la Servicejerarquía. Usaremos Services para formar la Cadena de Responsabilidad.

public abstract class Service {

    private Service next;

    public Service(Service next) {
        this.next = next;
    }

    public void handleRecurringTask(RecurringTask req) {
        if (next != null) {
            next.handleRecurringTask(req);
        }
    }
}

public class ExecuteService extends Service {

    public ExecuteService(Service next) {
        super(next);
    }

    void runTask(ExecuteTask task) {
        System.out.println(String.format("%s running %s with content '%s'", this.getClass().getSimpleName(),
                task.getClass().getSimpleName(), task.getName()));
    }

    public void handleRecurringTask(RecurringTask req) {
        if (!req.run(this)) {
            super.handleRecurringTask(req);
        }
    }
}

public class EmailService extends Service {

    public EmailService(Service next) {
        super(next);
    }

    public void runTask(SendEmailTask task) {
        System.out.println(String.format("%s running %s with content '%s'", this.getClass().getSimpleName(),
                task.getClass().getSimpleName(), task.getEmail()));
    }

    public void handleRecurringTask(RecurringTask req) {
        if (!req.run(this)) {
            super.handleRecurringTask(req);
        }
    }
}

La pieza final es la RecurringTaskSchedulerque organiza el proceso de carga y ejecución.

public class RecurringTaskScheduler{

    private List<RecurringTask> tasks = new ArrayList<>();

    private Service chain;

    public RecurringTaskScheduler() {
        chain = new EmailService(new ExecuteService(null));
    }

    public void loadTasks() {
        tasks.add(new SendEmailTask("here comes the first email"));
        tasks.add(new SendEmailTask("here is the second email"));
        tasks.add(new ExecuteTask("/root/python"));
        tasks.add(new ExecuteTask("/bin/cat"));
        tasks.add(new SendEmailTask("here is the third email"));
        tasks.add(new ExecuteTask("/bin/grep"));
    }

    public void runTasks(){
        for (RecurringTask task : tasks) {
            if (task.isOccuring(new Date())) {
                chain.handleRecurringTask(task);
            }
        }
    }
}

Ahora, aquí está la aplicación de ejemplo que demuestra el sistema.

public class App {

    public static void main(String[] args) {
        RecurringTaskScheduler scheduler = new RecurringTaskScheduler();
        scheduler.loadTasks();
        scheduler.runTasks();
    }
}

Ejecutando los resultados de la aplicación:

EmailService ejecutando SendEmailTask ​​con contenido 'aquí viene el primer correo electrónico'
EmailService ejecutando SendEmailTask ​​con contenido 'aquí está el segundo correo electrónico'
ExecuteService ejecutando ExecuteTask con contenido '/ root / python'
ExecuteService ejecutando ExecuteTask con contenido '/ bin / cat'
EmailService ejecutando SendEmailTask ​​con contenido 'aquí está el tercer correo electrónico'
ExecuteService que ejecuta ExecuteTask con contenido '/ bin / grep'

iluwatar
fuente
Puedo tener muchas tareas . Cada vez que agrego una nueva Tarea , necesito cambiar RecurringTask y también necesito cambiar todas sus subclases, porque necesito agregar una nueva función como public abstract boolean run (OtherService otherService) . Creo que Option4, el patrón de visitante que también implementa el envío doble tiene el mismo problema.
Sher10ck
Buen punto. Edité mi respuesta para que los métodos de ejecución (servicio) se definan en RecurringTask y devuelvan false por defecto. De esta manera, cuando necesita agregar otra clase de tarea, no necesita tocar las tareas relacionadas.
iluwatar