HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Using generic types for a custom mapper

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
genericmappercustomforusingtypes

Problem

I have a method which takes a certain generic object, which basically orders these objects based on a list of tags. Because I want to use this method for multiple objects, I have created a class which only contains the Id, list of tags and an average rating from each object, it looks like this:

public class RecommenderContentItem
{
    public Guid Id { get; set; }
    public List Tags { get; set; } //A tag is an object which only contains an Id and value, not relevant to the question.
    public double AverageRating { get; set; }
}


Now I have tried to create a generic method which takes T and checks which type it is, before converting it. The possible objects it can filter are Restaurant, AlgorithmRestaurant and Dish. All of these classes contain an Id, list of tags and an average. It looks like this:

```
public static List FilterOnContent(List ratedItems,
List itemsToFilter)
{
var cnv = new RciConverter();

var ratedRcis = new List();
var toFilterRcis = new List();
if (typeof(T) == typeof(Restaurant))
{
var rated = (IEnumerable)ratedItems;
var toFilter = (IEnumerable)itemsToFilter;

ratedRcis = cnv.ConvertMany(rated).ToList();
toFilterRcis = cnv.ConvertMany(toFilter).ToList();
}
if (typeof(T) == typeof(AlgorithmRestaurant))
{
var rated = (IEnumerable)ratedItems;
var toFilter = (IEnumerable)itemsToFilter;

ratedRcis = cnv.ConvertMany(rated).ToList();
toFilterRcis = cnv.ConvertMany(toFilter).ToList();
}
if (typeof(T) == typeof(Dish))
{
var rated = (IEnumerable)ratedItems;
var toFilter = (IEnumerable)itemsToFilter;

ratedRcis = cnv.ConvertMany(rated).ToList();
toFilterRcis = cnv.ConvertMany(toFilter).ToList();
}
if (!ratedRcis.Any() || !toFilterRcis.Any())
throw new TypeArgumentException("Invalid type."); //Custom exception written by Jon Skeet.

return ContentBasedFilter.Filter(ratedRc

Solution

Improving List FilterOnContent

You should use if/else structure when you need only a single condition to be triggered instead of multiple if's as the compiler will have to go over all of them, but that's not necessary because let's say typeof(T) == typeof(Restaurant) do you want to check typeof(T) == typeof(AlgorithmRestaurant) and the rest of your conditions?
There is no point the type argument cant be 2 types at the same time.

var rated = (IEnumerable)ratedItems;
var toFilter = (IEnumerable)itemsToFilter;


Those helper variables seem pointless to me they just add 2 extra lines in each if branch.

The .ToList() also adds a few extra characters, while you can just call it once at the return statement.

You currently have only if statements so you cant guarantee for the compiler that there will always be exactly one condition that will be met, which means you need to allocate some extra memory at initialization:

var ratedRcis = new List();


But with multiple else if's and a one else for the exception you wont need to do that.

The empty .Any() overload is O(1) operation but with else at the end you wont even need to do that.

With all of that in mind you can make your method look like this:

public static List FilterOnContent(List ratedItems, List itemsToFilter)
{
    RciConverter cnv = new RciConverter();

    IEnumerable ratedRcis;
    IEnumerable toFilterRcis;
    if (typeof(T) == typeof(Restaurant))
    {
        ratedRcis = cnv.ConvertMany((IEnumerable) ratedItems);
        toFilterRcis = cnv.ConvertMany((IEnumerable) itemsToFilter);
    }
    else if (typeof(T) == typeof(AlgorithmRestaurant))
    {
        ratedRcis = cnv.ConvertMany((IEnumerable) ratedItems);
        toFilterRcis = cnv.ConvertMany((IEnumerable) itemsToFilter);
    }
    else if (typeof(T) == typeof(Dish))
    {
        ratedRcis = cnv.ConvertMany((IEnumerable) ratedItems);
        toFilterRcis = cnv.ConvertMany((IEnumerable) itemsToFilter);
    }
    else
    {
        throw new TypeArgumentException("Invalid type."); //Custom exception written by Jon Skeet.
    }
    return ContentBasedFilter.Filter(ratedRcis.ToList(), toFilterRcis.ToList()).Select(rci => rci.Id).ToList();
}


In fact there isn't really a pretty way to shorten that, not much conversion is available to generics and since you need to know the type of casting at compile time you cant go fancy-mancy.

Shortening your mapping methods

Why would you write the same code in your ConvertMany when you already have it in the same method for a single instance Convert?

public IEnumerable ConvertMany(IEnumerable sourceObjects)
{
    return sourceObjects.Select(rest => new RecommenderContentItem
    {
        Id = rest.RestaurantId,
        AverageRating = rest.AverageRating,
        Tags = rest.Tags.ToList()
    });
}

public RecommenderContentItem Convert(AlgorithmRestaurant sourceObject)
{
    return new RecommenderContentItem
    {
        Id = sourceObject.RestaurantId,
        AverageRating = sourceObject.AverageRating,
        Tags = sourceObject.Tags.ToList()
    };
}


It doesn't makes sense you can just do:

public IEnumerable ConvertMany(IEnumerable sourceObjects)
{
    return sourceObjects.Select(rest => Convert(rest));
}


And even shorter with a method group:

public IEnumerable ConvertMany(IEnumerable sourceObjects)
{
    return sourceObjects.Select(Convert);
}


You can apply the same to all of your other ConvertMany methods.

Lastly I'm not sure why do you need everything to be in a List if you don't have a really good reason to use them, you shouldn't call .ToList() everywhere if you data is huge enumerating it again would surely cause a bottleneck there.

Code Snippets

var rated = (IEnumerable<AlgorithmRestaurant>)ratedItems;
var toFilter = (IEnumerable<AlgorithmRestaurant>)itemsToFilter;
var ratedRcis = new List<RecommenderContentItem>();
public static List<Guid> FilterOnContent<T>(List<T> ratedItems, List<T> itemsToFilter)
{
    RciConverter cnv = new RciConverter();

    IEnumerable<RecommenderContentItem> ratedRcis;
    IEnumerable<RecommenderContentItem> toFilterRcis;
    if (typeof(T) == typeof(Restaurant))
    {
        ratedRcis = cnv.ConvertMany((IEnumerable<Restaurant>) ratedItems);
        toFilterRcis = cnv.ConvertMany((IEnumerable<Restaurant>) itemsToFilter);
    }
    else if (typeof(T) == typeof(AlgorithmRestaurant))
    {
        ratedRcis = cnv.ConvertMany((IEnumerable<AlgorithmRestaurant>) ratedItems);
        toFilterRcis = cnv.ConvertMany((IEnumerable<AlgorithmRestaurant>) itemsToFilter);
    }
    else if (typeof(T) == typeof(Dish))
    {
        ratedRcis = cnv.ConvertMany((IEnumerable<Dish>) ratedItems);
        toFilterRcis = cnv.ConvertMany((IEnumerable<Dish>) itemsToFilter);
    }
    else
    {
        throw new TypeArgumentException("Invalid type."); //Custom exception written by Jon Skeet.
    }
    return ContentBasedFilter.Filter(ratedRcis.ToList(), toFilterRcis.ToList()).Select(rci => rci.Id).ToList();
}
public IEnumerable<RecommenderContentItem> ConvertMany(IEnumerable<AlgorithmRestaurant> sourceObjects)
{
    return sourceObjects.Select(rest => new RecommenderContentItem
    {
        Id = rest.RestaurantId,
        AverageRating = rest.AverageRating,
        Tags = rest.Tags.ToList()
    });
}

public RecommenderContentItem Convert(AlgorithmRestaurant sourceObject)
{
    return new RecommenderContentItem
    {
        Id = sourceObject.RestaurantId,
        AverageRating = sourceObject.AverageRating,
        Tags = sourceObject.Tags.ToList()
    };
}
public IEnumerable<RecommenderContentItem> ConvertMany(IEnumerable<AlgorithmRestaurant> sourceObjects)
{
    return sourceObjects.Select(rest => Convert(rest));
}

Context

StackExchange Code Review Q#153121, answer score: 2

Revisions (0)

No revisions yet.