views:

145

answers:

5

Hello chaps

I have two almost identical c# functions. Because they're so similar I thought I'd try out generics, but I'm stumped on how to do it. Any suggestions, or am I barking up the wrong tree entirely?

    public IList<UnitTemplate> UnitTemplates { get; set; }
    public IList<QualTemplate> QualTemplates { get; set; }

    public QualTemplate FindQualTemplate(string templateID)
    {
        QualTemplate selectedQualTemplate;
        if (QualTemplates.Count == 0)
            throw new CreatioException("This user's brand has no QualTemplates. There must be at least one available.");
        if (QualTemplates.Count == 1 || String.IsNullOrEmpty(templateID))
            selectedQualTemplate = QualTemplates.First();
        else
            selectedQualTemplate = QualTemplates.Single(x => x.QualTemplateID.ToLower() == templateID.ToLower());
        if (selectedQualTemplate == null)
            throw new CreatioException(String.Format("No QualTemplate with the id {0} could be found for this user's brand.", templateID));
        return selectedQualTemplate;
    }

    public UnitTemplate FindUnitTemplates(string templateID)
    {
        UnitTemplate selectedTemplate;
        if (UnitTemplates.Count == 0)
            throw new CreatioException("This user's brand has no UnitTemplates. There must be at least one available.");
        if (UnitTemplates.Count == 1 || String.IsNullOrEmpty(templateID))
            selectedTemplate = UnitTemplates.First();
        else
            selectedTemplate = UnitTemplates.Single(x => x.UnitTemplateID.ToLower() == templateID.ToLower());
        if (selectedTemplate == null)
            throw new CreatioException(String.Format("No UnitTemplate with the id {0} could be found for this user's brand.", templateID));
        return selectedTemplate;
    }
+1  A: 

This is not easily possible. UnitTemplates and QualTemplates need to implement the same interface or have the same base class that exposes the members you are using and the same goes for QualTemplate and UnitTemplate. Even then, you would need to pass the factory to use to the method (e.g. UnitTemplates as a parameter) and you would need to specify the predicate you are using in the Single call externally (since the ID property name is different).

testalino
+7  A: 

The problem you have is that both methods use a property which the two types don’t have in common: QualTemplateID and UnitTemplateID. If you can make the following changes to the code structure:

  • Declare UnitTemplate and QualTemplate to derive from a common base type, Template

  • In that base type, declare a property TemplateID

  • Get rid of QualTemplateID and UnitTemplateID and use the inherited TemplateID property instead

then you can write the method generically:

public TTemplate FindTemplates<TTemplate>(
    IList<TTemplate> templates, string templateID)
    where TTemplate : Template
{
    TTemplate selectedTemplate;
    if (templates.Count == 0)
        throw new CreatioException("This user's brand has no template. There must be at least one available.");
    if (templates.Count == 1 || String.IsNullOrEmpty(templateID))
        selectedTemplate = templates.First();
    else
        selectedTemplate = templates.Single(x => x.TemplateID.ToLower() == templateID.ToLower());
    return selectedTemplate;
}

I’ve removed the if (selectedTemplate == null) because it would never fire anyway (unless the list is likely to contain nulls, but then the predicate you pass to Single would crash...).

The above works equally well if you make it an interface instead of a base type.

If you cannot make the changes to the code that I described, then your only option is to pass (as a parameter) a delegate that retrieves the ID:

public TTemplate FindTemplates<TTemplate>(
    IList<TTemplate> templates, string templateID,
    Func<TTemplate, string> templateIdGetter)
{
    TTemplate selectedTemplate;
    if (templates.Count == 0)
        throw new CreatioException("This user's brand has no template. There must be at least one available.");
    if (templates.Count == 1 || String.IsNullOrEmpty(templateID))
        selectedTemplate = templates.First();
    else
        selectedTemplate = templates.Single(x => templateIdGetter(x).ToLower() == templateID.ToLower());
    return selectedTemplate;
}

var qTempl = FindTemplates(QualTemplates, "myTemplateId", q => q.QualTemplateID);
var uTempl = FindTemplates(UnitTemplates, "myTemplateId", u => u.UnitTemplateID);
Timwi
Awesome answer, Timwi. Thanks a lot! I can make that code change so that seems very neat.
centralscru
+6  A: 

I would use an interface, like:

public interface ITemplate
{
    string TemplateId { get; }
}

with explicit implementation:

public class UnitTemplate : ITemplate {
    public string UnitTemplateID { get; set; }
    string ITemplate.TemplateId { get { return UnitTemplateID; } }
}
public class QualTemplate : ITemplate
{
    public string QualTemplateID { get; set; }
    string ITemplate.TemplateId { get { return QualTemplateID; } }
}

Then I can write a generic extension method to handle this for any T:

public static class TemplateExtensions
{
    public static T Find<T>(this ICollection<T> templates, string templateID) where T : ITemplate
    {
        T selectedTemplate;
        if (templates.Count == 0)
            throw new CreatioException("This user's brand has no templates. There must be at least one available.");
        if (templates.Count == 1 || String.IsNullOrEmpty(templateID))
            selectedTemplate = templates.First();
        else
            selectedTemplate = templates.Single(x => x.TemplateId.ToLower() == templateID.ToLower());
        if (selectedTemplate == null)
            throw new CreatioException(String.Format("No template with the id {0} could be found for this user's brand.", templateID));
        return selectedTemplate;
    }
}

And finally, proxy those methods:

public QualTemplate FindQualTemplate(string templateID)
{
    return QualTemplates.Find(templateID);
}
public UnitTemplate FindUnitTemplates(string templateID)
{
    return UnitTemplates.Find(templateID);
}

If you want to avoid an interface, you can use a selector instead:

public QualTemplate FindQualTemplate(string templateID)
{
    return Find(QualTemplates, templateID, x => x.QualTemplateID);
}
public UnitTemplate FindUnitTemplates(string templateID)
{
    return Find(UnitTemplates, templateID, x => x.UnitTemplateID);
}
static T Find<T>(ICollection<T> templates, string templateID, Func<T, string> selector)
{
    T selectedTemplate;
    if (templates.Count == 0)
        throw new CreatioException("This user's brand has no templates. There must be at least one available.");
    if (templates.Count == 1 || String.IsNullOrEmpty(templateID))
        selectedTemplate = templates.First();
    else
        selectedTemplate = templates.Single(x => selector(x).ToLower() == templateID.ToLower());
    if (selectedTemplate == null)
        throw new CreatioException(String.Format("No template with the id {0} could be found for this user's brand.", templateID));
    return selectedTemplate;
}
Marc Gravell
Thanks very much for your detailed answer - very helpful indeed.
centralscru
A: 

This would be the simplest solution I could see, and you can even do it as an extension method if you want. (If you don't just remove the static and this piece)

public static class Extension {

     public static T FindTemplates<T>(this IList<T> list, string templateID, Func<string,T> selector) 
       {
        T selectedTemplate;
        if (list.Count == 0){
            throw new CreationException("This user's brand has no UnitTemplates. There must be at least one available.");
        }
        if (list.Count == 1 || String.IsNullOrEmpty(templateID)){
            selectedTemplate =  list.First();
        }
        else{
            selectedTemplate = selector(templateID); 
        }
        if (selectedTemplate == null){
            throw new CreationException(String.Format("No UnitTemplate with the id {0} could be found for this user's brand.", templateID));
        }
        return selectedTemplate;
    }
  }

Then usage would be like this:

IList<UnitTemplate> test = new List<UnitTemplate>();
UnitTemplate t = test.FindTemplates("id", (string x) => test.Single(y => y.UnitTemplateID.ToLower() == x.ToLower()));
Nix
Please explain whats wrong... because it works.
Nix
I do agree the best way is to abstract a base type like @Timwi mentioned. But this is an alternative way to do it without having to change anything.
Nix
+3  A: 

You can use a Func:

public IList<UnitTemplate> UnitTemplates { get; set; }
public IList<QualTemplate> QualTemplates { get; set; }

public QualTemplate FindQualTemplate(string templateID)
{
    return FindTemplatesImpl(templateID, x => x.QualTemplateID, QualTemplates );
}

public UnitTemplate FindUnitTemplates(string templateID)
{
    return FindTemplatesImpl(templateID, x => x.UnitTemplateID, UnitTemplates );
}

public T FindTemplatesImpl<T>(string templateID, Func<T, string> expr, IList<T> templates)
{
    T selectedTemplate;
    if (templates.Count == 0)
        throw new CreatioException("This user's brand has no Templates. There must be at least one available.");

    if (templates.Count == 1 || String.IsNullOrEmpty(templateID))
        selectedTemplate = templates.First();
    else
        selectedTemplate = templates.Single(x => expr(x).ToLower() == templateID.ToLower());

    if (selectedTemplate == null)
        throw new CreatioException(String.Format("No UnitTemplate with the id {0} could be found for this user's brand.", templateID));

    return selectedTemplate;
}
mathieu