Thursday, May 5, 2011

How to refactor these 2 similar methods into one?

Hello,

I've seen some samples of using 'T' to make a method reuseable for generic collections of different classes, but I've never really gotten into it or understood the samples.

I wonder if it would be possible to put the 2 methods below into one and what the downsides of doing this would be (performance-wise).

Anyone?

        [NonAction]
        public List<SelectListItem> ToSelectList(IEnumerable<Department> departments, string defaultOption)
        {
            var items = departments.Select(d => new SelectListItem() { Text = d.Code + " - " + d.Description, Value = d.Id.ToString() }).ToList();
            items.Insert(0, new SelectListItem() { Text = defaultOption, Value = "-1" });
            return items;
        }

        [NonAction]
        public List<SelectListItem> ToSelectList(IEnumerable<Function> functions, string defaultOption)
        {
            var items = functions.Select(f => new SelectListItem() { Text = f.Description, Value = f.Id.ToString() }).ToList();
            items.Insert(0, new SelectListItem() { Text = defaultOption, Value = "-1" });
            return items;
        }


SOLUTION

The solution that I used:

usage

var departmentItems = departments.ToSelectList(d => d.Code + " - " + d.Description, d => d.Id.ToString(), " - ");
var functionItems = customerFunctions.ToSelectList(f => f.Description, f => f.Id.ToString(), " - ");

with

 public static class MCVExtentions
    {
        public static List<SelectListItem> ToSelectList<T>(this IEnumerable<T> enumerable, Func<T, string> text, Func<T, string> value, string defaultOption)
        {
            var items = enumerable.Select(f => new SelectListItem() { Text = text(f), Value = value(f) }).ToList();
            items.Insert(0, new SelectListItem() { Text = defaultOption, Value = "-1" });
            return items;
        }
    }
From stackoverflow
  • The old school way would be to create a common interface for both Department and Function:

    interface A
    {
    int ID{get;}
    string Description{get;}
    }
    

    You implement Description on Department to return d.Code + " - " + d.Description. and write the function to use this interface instead of concrete classes:

    [NonAction]
        public List<SelectListItem> ToSelectList(IEnumerable<A> as, string defaultOption)
        {
            var items = as.Select(a => new SelectListItem() { Text = a.Description, Value = a.Id.ToString() }).ToList();
            items.Insert(0, new SelectListItem() { Text = defaultOption, Value = "-1" });
            return items;
        }
    

    EDIT: Regarding using generics, its not going to help much in this case, because

    • the objects you are passing needs to implement Id and Description
    • you are not returning these objects, so in this respect you don't have to care about type safety of generics
    Thomas Stock : Ofcourse! Thanks. I was too stuck thinking about those samples with 'T' that I didn't realize an interface was all I needed. Thanks a lot.
    Svish : I would say using generics and functions would be a better thing to do in this case. Then you don't have to force a bunch of classes into implementing an interface. You might want to use that ToSelectList function on a class that does not have an ID or Description, and where adding it would be not very logical (or for example the Description property should really be called something else).
    Thomas Stock : Thanks Svish. Good point.
  • In fact you can do it with a combination of generics and functions, something along the lines of this (untested may not even compile).

    [NonAction]
    public List<SelectListItem> ToSelectList<T>(IEnumerable<T> en, 
                                                Function<string, T> text, 
                                                Function<string, T> value, 
                                                string defaultOption)
    {
        var items = en.Select(x => new SelectListItem() { Text = text(x) , Value = value(x) }).ToList();
        items.Insert(0, new SelectListItem() { Text = defaultOption, Value = "-1" });
        return items;
    }
    

    Then you can dispatch to it with the appropriate lambda functions (or call directly).

    [NonAction]
    public List<SelectListItem> ToSelectList(IEnumerable<Department> departments, 
                                             string defaultOption)
    {
        return ToSelectList<Department>(departments, d =>  d.Code + '-' + d.Description, d => d.Id.ToString(), defaultOption);
    
    }
    
  • Without implementiong a common interface like @Grzenio suggested, you could use a generic method like this:

        public List<SelectListItem> ToSelectList<T>(IEnumerable<T> enumerable, Func<T, string> text, Func<T, string> value, string defaultOption)
        {
            var items = enumerable.Select(f => new SelectListItem() { Text = text(f), Value = value(f) }).ToList();
            items.Insert(0, new SelectListItem() { Text = defaultOption, Value = "-1" });
            return items;
        }
    
        // use like
    
        t.ToSelectList(departments, d => d.Code + " - " + d.Description, d => d.Id.ToString(), "default");
        t.ToSelectList(functions, f => f.Description, f => f.Id.ToString(), "default");
    
    Thomas Stock : Thanks! This was what I was looking for in the first place. I'll try to implement yours and see if I like working with it
    Motti : Hey my answer is identical and posted three minutes before this answer yet this answer gets 3 votes and mine zero! OK I'll up-vote it anyway, if only for having the generic parameters for Function in the right order...
    idursun : ToSelectList can also be made an extension method.
    Thomas Stock : I'm sorry Motti, but I found your answer to be less clear. I immediatly understood bruno's post because of the usage examples. Will upvote yours now.
    Thomas Stock : I implemented this with an extension method and it looks and feels great! thanks for the help, everybody.

0 comments:

Post a Comment