Refactoring di tre classi molto simili usando l'ereditarietà?

Il problema è che il design dell'API sottostante ama per ripetere le informazioni sul tipo in tutti i nomi di proprietà:

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

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

Questo misdesign rende difficile l'astrazione di tutti gli oggetti Api.X altrimenti identici. In particolare, i tuoi metodi differiscono in questi punti:

• l'oggetto Api.X su cui sono chiamati i metodi
• il nome del metodo Api.X.GetX() per ottenere la risposta iniziale
• il nome della proprietà x.X che accede ai dati in una risposta
• il tipo di risposta da GetPageByUrl

La soluzione è di introdurre uno strato appropriato che elimini queste differenze. Per esempio. potresti definire una funzione comune CommonGetData che potrebbe essere invocata in questo modo: ^[1]

^{[1]: Si noti che non sono fluente in C #, quindi prendi nota del design concettuale piuttosto che delle specifiche della sintassi.}

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

Con CommonGetData definito in modo approssimativo come questo:

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;
}

Se l'uso di lambda non è il tuo caso, potresti anche notare che questo sembra molto simile a un candidato per il modello di strategia (in CommonGetData , i parametri di callback rappresentano strategie) o per il modello di metodo del modello. Potremmo quindi definire una Api astratta come questa:

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);
    }
}

che verrebbe usato come

private UsersApi usersApi = new UsersApi();

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

Idealmente, le API esistenti potrebbero essere sottoposte a refactoring per implementare interfacce comuni, in modo che il loro inserimento in interfacce comuni che delegano i metodi incompatibili non sarebbe necessario.

Si noti inoltre che l'uso molto elevato delle classi statiche probabilmente intralcia il riutilizzo del codice, poiché rende difficile il passaggio di questi valori come valori. Se esiste una sola istanza, preferisci il Pattern Singleton che rende anche le tue classi più facili da testare rispetto alle classi statiche.

Ciò di cui hai bisogno è una funzione di helper generica non membro (utilità statica o classe base) per concatenare i risultati di NextPage da diversi oggetti IEnumerable .

Una volta fatto, circa cinque righe di codice (in ogni classe) possono essere ridotte a una riga. Ciò eliminerà il disordine e IMHO non richiede ulteriori refactoring.

Sarebbe meglio se NextPage fosse un metodo di interfaccia sull'oggetto risposta. Allo stesso modo, sarebbe meglio se l'oggetto risposta implementasse IEnumerable<T> per il rispettivo tipo di ritorno T .

In caso contrario, puoi prendere in considerazione:

• Correggere la libreria o il codice upstream
• Utilizza digita dinamico
  • Ma questo è un percorso molto sfuggente.

Se veramente vuoi refactoring queste classi usando l'ereditarietà, allora va bene, vai avanti, la risposta di amon è giusta sul denaro.

Tuttavia, si tenga presente che l'eliminazione di una piccola duplicazione del codice è una delle ragioni meno legittime per complicare un progetto utilizzando l'ereditarietà. Se il codice risultante è più dettagliato dell'originale, si avrà una perdita netta di leggibilità, comprensibilità e manutenibilità.

Potrebbe essere effettivamente peggio di un po 'di duplicazione del codice.

Potresti creare una classe generica con una variabile di tipo per la classe di dati variabile?

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

Le classi specifiche annullerebbero solo il metodo di definizione del tempo.

Dopo aver esaminato alcune delle informazioni pubblicate da @Amon, penso di avere qualcosa di un po 'più ordinato. È più prolisso, ma sembra più pulito.

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);
        }
    }
}

Penso che sia possibile ridurre ZendeskCache ancora un po ', ma continuerò ad armeggiare. Sto cercando di capire i metodi per rendere le mie pratiche di codifica future più pulite e più manutenibili.