Search code examples
c#genericsreflectiondrycode-readability

Improving code with reflection and generics


With the following code, how could I improve its readability using reflection and generics? It's a lot of repeated code, but I'm new to these concepts. "dl" is an interface but I'm not sure why I can't get access to the properties I want with .getType().getProperties(), it returns an empty list.

if (entity == "SERVICE")
{
    if (filter == "Active == 1")
    {
        foreach (var service in dl.Services.List().Where(x => x.Active).OrderBy(x => x.Name))
            tmpReturn.Add(service.ID, service.Name);

        return tmpReturn;
    }

    foreach (var service in dl.Services.List().OrderBy(x => x.Name))
        tmpReturn.Add(service.ID, service.Name);

    return tmpReturn;
}

if (entity == "CAMPAIGN")
{
    if (filter == "Active == 1")
    {
        foreach (var campaign in dl.Campaigns.List().Where(x => x.Active).OrderBy(x => x.Name))
            tmpReturn.Add(campaign.ID, campaign.Name);

        return tmpReturn;
    }

    foreach (var campaign in dl.Campaigns.List().OrderBy(x => x.Name))
        tmpReturn.Add(campaign.ID, campaign.Name);

    return tmpReturn;
}

if (entity == "AGENT")
{
    if (condition == "Active != ")
    { 
        if (value == "1")
        { 
            foreach (var agent in dl.Agents.List().OrderBy(x => x.Name))
                tmpReturn.Add(agent.ID, agent.Name);

            return tmpReturn;
        }

        foreach (var agent in dl.Agents.List().Where(x => x.Active).OrderBy(x => x.Name))
            tmpReturn.Add(agent.ID, agent.Name);

        return tmpReturn;
    }
    else if (condition == "Active == ")
    {
        if (value == "1")
        {
            foreach (var agent in dl.Agents.List().Where(x => x.Active).OrderBy(x => x.Name))
                tmpReturn.Add(agent.ID, agent.Name);

            return tmpReturn;
        }

        foreach (var agent in dl.Agents.List().OrderBy(x => x.Name))
            tmpReturn.Add(agent.ID, agent.Name);

        return tmpReturn;
    }
}

Solution

  • Since the different entities do not implement a common interface, we use ValueTuples as intermediate result. I am not sure what the ID type is. I am assuming int here, but you can change this easily.

    IEnumerable<(int ID, string Name, bool Active)> source;
    bool filterByActive;
    switch (entity) {
        case "SERVICE":
            source = dl.Services.List().Select(x => (x.ID, x.Name, x.Active));
            filterByActive = filter == "Active == 1";
            break;
        case "CAMPAIGN":
            source = dl.Campaigns.List().Select(x => (x.ID, x.Name, x.Active));
            filterByActive = filter == "Active == 1";
            break;
        case "AGENT":
            source = dl.Agents.List().Select(x => (x.ID, x.Name, x.Active));
            filterByActive = condition == "Active != " && value != "1" ||
                             condition == "Active == " && value == "1";
            break;
        default:
            throw new NotImplementedException();
    }
    if (filterByActive) {
        source = source.Where(x => x.Active);
    }
    return source
        // .OrderBy(x => x.Name)       Not for Dictionary!
        .ToDictionary(x => x.ID, x => x.Name);
    

    The idea is that we return an enumeration representing the data source and a Boolean telling whether we have to filter in a switch statement. All the other things can then be performed on this data source.

    We can compose the enumerable by applying the Where separately.

    Using the LINQ extension method ToDictionary() instead of the addition of single elements in a loop simplifies things further. Note that if you are adding things to a dictionary, sorting the items has no effect.