¿Está refactorizando tres clases muy similares usando la herencia?

7

Actualmente estoy trabajando en la refactorización de la base de códigos para uno de nuestros servicios. He estado revisando todo, y siento que está un poco disperso, y probablemente podría adherirme mejor a los principios de OOP.

Tengo tres clases que son todas derivadas de otra clase Cache . Las tres de estas clases realizan exactamente las mismas operaciones, la única diferencia es el tipo de objeto que están consultando y qué método llaman para consultarlas. ¿Cuál sería el mejor método para hacer que estas clases sean aún más simples?

public static class ZenDeskCache
{
    public static ZendeskApi Api = new ZendeskApi(GlobalVariables.ZendeskUrl, GlobalVariables.ZendeskUser,
        GlobalVariables.ZendeskPass);

    public class Users : Cache<Users, List<User>>
    {
        protected override List<User> GetData()
        {
            var users = Api.Users.GetAllUsers();
            var allUsers = new List<User>(users.Users);

            while (users.NextPage != null)
            {
                users = Api.Users.GetByPageUrl<GroupUserResponse>(users.NextPage);
                allUsers.AddRange(new List<User>(users.Users));
            }

            allUsers = allUsers.OrderBy(n => n.Name).ToList();

            return allUsers;
        }

        protected override TimeSpan GetLifetime()
        {
            return TimeSpan.FromDays(1);
        }
    }

    public class Organizations : Cache<Organizations, List<Organization>>
    {
        protected override List<Organization> GetData()
        {
            var organizations = Api.Organizations.GetOrganizations();
            var allOrgs = new List<Organization>(organizations.Organizations);

            while (organizations.NextPage != null)
            {
                organizations = Api.Users.GetByPageUrl<GroupOrganizationResponse>(organizations.NextPage);
                allOrgs.AddRange(new List<Organization>(organizations.Organizations));
            }

            allOrgs = allOrgs.OrderBy(n => n.Name).ToList();

            return allOrgs;
        }

        protected override TimeSpan GetLifetime()
        {
            return TimeSpan.FromDays(1);
        }
    }

    public class Groups : Cache<Groups, List<Group>>
    {
        protected override List<Group> GetData()
        {
            var groups = Api.Groups.GetGroups();
            var allGroups = new List<Group>(groups.Groups);

            while (groups.NextPage != null)
            {
                groups = Api.Groups.GetByPageUrl<MultipleGroupResponse>(groups.NextPage);
                allGroups.AddRange(new List<Group>(groups.Groups));
            }

            allGroups = allGroups.OrderBy(n => n.Name).ToList();

            return allGroups;
        }

        protected override TimeSpan GetLifetime()
        {
            return TimeSpan.FromSeconds(600);
        }
    }
}
    
pregunta JD Davis 19.11.2015 - 18:29

5 respuestas

5

El problema es que el diseño de la API subyacente ama para repetir la información de tipo en todos los nombres de propiedades:

... Api.Groups.GetGroups();
... groups.Groups ...

    ... Api.Groups.GetByPageUrl<MultipleGroupResponse>(...);
    ... groups.Groups ...

Este diseño erróneo hace que sea difícil abstraer todos los objetos Api.X idénticos. En particular, sus métodos difieren en estos puntos:

  • el objeto Api.X sobre el que se invocan los métodos
  • el nombre del método Api.X.GetX() para obtener la respuesta inicial
  • el nombre de la propiedad x.X que accede a los datos en una respuesta
  • el tipo de respuesta de GetPageByUrl

La solución es introducir una capa apropiada que nivela estas diferencias. P.ej. podría definir una función común CommonGetData que podría invocarse así: [1]

[1]: tenga en cuenta que no domino C #, por lo que solo tome nota del diseño conceptual en lugar de los detalles de la sintaxis.

protected override List<User> GetData()
{
    return CommonGetData(
        Api.Users,
        (api) => api.GetAllUsers(),
        (response) => response.Users,
    );
}

Con CommonGetData definido aproximadamente así:

private static <T, ResponseT, ApiT> List<T> CommonGetData(
        ApiT api,
        Function<ResponseT, ApiT> getInitialResponse,
        Function<Collection<T>, ResponseT> itemsFromResponse,
    )
{
    ResponseT response = getInitialResponse(api);
    var allItems = new List<T>(itemsFromResponse(response);

    while (response.NextPage != null)
    {
        response = api.GetPageByUrl<ResponseT>(response.NextPage);
        allItems.AddRange(new List<T>(itemsFromResponse(response));
    }

    allItems = allItems.OrderBy(n => n.Name).ToList();

    return allItems;
}

Si no te gusta utilizar lambdas, también podrías tener en cuenta que se parece mucho a un candidato para el Patrón de estrategia (en CommonGetData , los parámetros de devolución de llamada representan estrategias) o para el patrón de método de plantilla. Entonces podríamos definir una Api abstracta como esta:

abstract class CommonApi<T, ResponseT>
{
    protected abstract ResponseT GetInitialResponse();
    protected abstract Collection<T> ItemsFromResponse(ResponseT response);
    protected abstract GetPageByUrl(Url url);

    private List<T> GetData()
    {
        ResponseT response = GetInitialResponse();
        var allItems = new List<T>(ItemsFromResponse(response);

        while (response.NextPage != null)
        {
            response = GetPageByUrl(response.NextPage);
            allItems.AddRange(new List<T>(ItemsFromResponse(response));
        }

        allItems = allItems.OrderBy(n => n.Name).ToList();

        return allItems;
    }
}

class UsersApi : CommonApi<User, MultipleUserResponse>
{
    protected override MultipleUserResponse GetInitialResponse()
    {
        return Api.Users.GetAllUsers();
    }

    protected override Collection<User> ItemsFromResponse(MultipleUserResponse response)
    {
        return response.Users;
    }

    protected override GetPageByUrl(Url url)
    {
        Api.Users.GetPageByUrl<MultipleUserResponse>(url);
    }
}

que se usaría como

private UsersApi usersApi = new UsersApi();

protected override List<User> GetData()
{
    return usersApi.GetData();
}

Idealmente, las API existentes se podrían refactorizar para implementar interfaces comunes, de modo que no sería necesario envolverlas en interfaces comunes que delegan a los métodos incompatibles.

También tenga en cuenta que su uso muy elevado de clases estáticas probablemente interfiere con la reutilización del código, ya que hace que sea difícil pasarlos como valores. Si solo puede existir una instancia, prefiera el patrón Singleton, que también hace que sus clases sean más fáciles de probar en comparación con las clases estáticas.

    
respondido por el amon 19.11.2015 - 20:35
2

Lo que necesita es una función auxiliar genérica que no sea miembro (utilidad estática o clase base) para concatenar los resultados NextPage de diferentes objetos IEnumerable .

Una vez hecho esto, aproximadamente cinco líneas de código (en cada clase) se pueden condensar en una sola línea. Esto despejará el desorden y, en mi humilde opinión, no es necesaria una refactorización adicional.

Sería mejor si NextPage es un método de interfaz en el objeto de respuesta. Del mismo modo, sería mejor si el objeto de respuesta implementara IEnumerable<T> para el tipo de devolución respectivo T .

Si ese no es el caso, puede considerar:

  • Corregir la biblioteca o el código en sentido ascendente
  • Utilice tipo dinámico
    • Pero este es un camino muy resbaladizo.
respondido por el rwong 19.11.2015 - 20:02
1

Si realmente quiere refactorizar estas clases usando la herencia, entonces está bien, adelante, la respuesta de Amon está justo en el dinero.

Sin embargo, tenga en cuenta que eliminar una pequeña duplicación de código es una de las razones menos legítimas para complicar un diseño mediante el uso de la herencia. Si el código resultante es más detallado que el original, tiene una pérdida neta de legibilidad, comprensión y facilidad de mantenimiento.

Esto podría ser peor que un poco de duplicación de código.

    
respondido por el Mike Nakis 20.11.2015 - 14:50
0

¿Podría crear una clase genérica con una variable de tipo para la clase de datos variable?

public class WhateverCache<T> : Cache<T, List<T>> {
  protected override List<T> GetData() {
    // whatever logic you have
  }

Las clases específicas solo anularían el método que define el intervalo de tiempo.

    
respondido por el 9000 19.11.2015 - 18:44
0

Después de revisar parte de la información publicada por @Amon, creo que tengo algo un poco mejor. Es más detallado, pero parece ser más limpio.

CommonApi

public abstract class CommonApi<T, TResponse>
    where TResponse : GroupResponseBase
{
    protected abstract TResponse GetInitialResponse();
    protected abstract List<T> ItemsFromResponse(TResponse response);
    protected abstract TResponse GetPageByUrl(string url);

    public ZendeskApi Api = new ZendeskApi(
        GlobalVariables.ZendeskUrl,
        GlobalVariables.ZendeskUser,
        GlobalVariables.ZendeskPass);

    public List<T> GetData()
    {
        var response = GetInitialResponse();
        var allItems = new List<T>(ItemsFromResponse(response));

        while (response.NextPage != null)
        {
            response = GetPageByUrl(response.NextPage);
            allItems.AddRange(ItemsFromResponse(response));
        }

        allItems = SortData(allItems);

        return allItems;
    }

    private List<T> SortData(List<T> list)
    {
        return list;
    }

    public List<User> SortData(List<User> list)
    {
        return list.OrderBy(n=>n.Name).ToList();
    }

    public List<Group> SortData(List<Group> list)
    {
        return list.OrderBy(n => n.Name).ToList();
    }

    public List<Organization> SortData(List<Organization> list)
    {
        return list.OrderBy(n => n.Name).ToList();
    }
}

UserListApi

public class UserListApi : CommonApi<User, GroupUserResponse>
{
    protected override GroupUserResponse GetInitialResponse()
    {
        return Api.Users.GetAllUsers();
    }

    protected override List<User> ItemsFromResponse(GroupUserResponse response)
    {
        return response.Users.ToList();
    }

    protected override GroupUserResponse GetPageByUrl(string url)
    {
        return Api.Users.GetByPageUrl<GroupUserResponse>(url);
    }
}

OrganizationListApi

public class OrganizationListApi : CommonApi<Organization, GroupOrganizationResponse>
{
    protected override GroupOrganizationResponse GetInitialResponse()
    {
        return Api.Organizations.GetOrganizations();
    }

    protected override List<Organization> ItemsFromResponse(GroupOrganizationResponse response)
    {
        return response.Organizations.ToList();
    }

    protected override GroupOrganizationResponse GetPageByUrl(string url)
    {
        return Api.Organizations.GetByPageUrl<GroupOrganizationResponse>(url);
    }
}

GroupListApi

public class GroupListApi : CommonApi<Group, MultipleGroupResponse>
{
    protected override MultipleGroupResponse GetInitialResponse()
    {
        return Api.Groups.GetGroups();
    }

    protected override List<Group> ItemsFromResponse(MultipleGroupResponse response)
    {
        return response.Groups.ToList();
    }

    protected override MultipleGroupResponse GetPageByUrl(string url)
    {
        return Api.Groups.GetByPageUrl<MultipleGroupResponse>(url);
    }
}

ZendeskCache

public class ZendeskCache
{
    public class Users : Cache<Users, List<User>>
    {
        protected override List<User> GetData()
        {
            var users = new UserListApi();

            return users.GetData();
        }

        protected override TimeSpan GetLifetime()
        {
            return TimeSpan.FromDays(1);
        }
    }

    public class Organizations : Cache<Organizations, List<Organization>>
    {
        protected override List<Organization> GetData()
        {
            var orgs = new OrganizationListApi();

            return orgs.GetData();
        }

        protected override TimeSpan GetLifetime()
        {
            return TimeSpan.FromDays(1);
        }
    }

    public class Groups : Cache<Groups, List<Group>>
    {
        protected override List<Group> GetData()
        {
            var groups = new GroupListApi();
            return groups.GetData();
        }

        protected override TimeSpan GetLifetime()
        {
            return TimeSpan.FromSeconds(1);
        }
    }
}

Creo que puede ser posible reducir el ZendeskCache incluso un poco más, pero voy a seguir jugando. Estoy intentando descubrir métodos para hacer que mis futuras prácticas de codificación sean más limpias y más fáciles de mantener.

    
respondido por el JD Davis 20.11.2015 - 16:28

Lea otras preguntas en las etiquetas