patterncsharpMinor
Using generic types for a custom mapper
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:
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
```
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
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
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
There is no point the type argument cant be 2 types at the same time.
Those helper variables seem pointless to me they just add 2 extra lines in each
The
You currently have only
But with multiple
The empty
With all of that in mind you can make your method look like this:
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
It doesn't makes sense you can just do:
And even shorter with a method group:
You can apply the same to all of your other
Lastly I'm not sure why do you need everything to be in a
List FilterOnContentYou 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.