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

Switch which object is being retrieved from the database based on name

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

Problem

I have a generic Get function that returns a list of objects based on the name of the desired type of object. This has let to an enormous case/switch statement that delegates out to the specific Get functions for the given type. I'd like use reflection or something similar to avoid this huge case/switch. I can't directly cast from the child types to Record.

Here's the case/switch

```
public static IEnumerable GetChildren(string tenantName, string parentCollection, string parentId,
string childCollection)
{
M5Record parent;
try
{
parent = GetById(tenantName, parentCollection, parentId);
}
catch (InvalidOperationException)
{
throw new WebFaultException(
new ErrorResponse { Error = "Not found" }, HttpStatusCode.NotFound);
}
childCollection = childCollection.ToLower();
if (parent == null)
return new List();
switch (childCollection)
{
case "assets":
case "asset":
return parent.GetAssets();
case "applications":
case "application":
return parent.GetApplications();
case "appointment":
return parent.GetAppointments();
case "bonds":
case "bond":
return parent.GetBonds();
case "buildings":
case "building":
return parent.GetBuildings();
case "certificates":
case "certificate":
return parent.GetCertificates();
case "checklistitems":
case "checklistitem":
return parent.GetCheckListItems();
case "companies":
case "company":
return parent.GetCompanies();
case "complaints":
case "complaint":
return parent.GetComplaints();
case "condition":
return parent.GetConditions();
case "contacts":
case "contact":
return parent.GetContacts();
case "courtappearance":
return parent.GetCourtAppearanc

Solution

The first thing I would say is that the API is not well-defined. The upstream code should be calling the specific method. The fact that you call a single method based on multiple names is fragile. It seems like you are trying to stick an entire API into a single method.

It would be much better if you exposed one method per "childCollection", for example:

//This method is exposed as part of the web API
public static GetAssets(string tenantName, string parentCollection, string parentId)
{
    var parent = GetParent(tenantName, parentCollection, parentId);

    if (parent == null)
        return new List();

    //...
}

private static M5Record GetParent(string tenantName, string parentCollection, string parentId) 
{
    try
    {
        return GetById(tenantName, parentCollection, parentId);
    }
    catch (InvalidOperationException)
    {
        throw new WebFaultException(
            new ErrorResponse { Error = "Not found" }, HttpStatusCode.NotFound);
    }
}


And so on for each one of your childCollection cases. This gives you much more flexibility when extending your API or even adjusting what parameters go to what methods (for example, the stockpiletransaction case).

Another thing it gives you is not having to worry about the locale of the text passed in for the switch statement. You are assuming English but what if somebody tried using your API in Spanish? Using a well-defined API forces them to call concrete methods instead of methods by text name.

If you really are stuck here, you can use a delegate "registration" method. For example:

public class MyWebAPI
{
    private static Dictionary>> _registry;

    static MyWebAPI()
    {
        _registry = new Dictionary>>(StringComparer.OrdinalIgnoreCase);
        _registry["asset"] = (p) => { return p.GetAssets(); };
        _registry["assets"] = _registry["asset"];
        //... etc
    }

    public static MyMethod(string methodName)
    {
        var parent = ...;
        _registry[methodName](parent);
    }
}


Which allows you to easily add cases. You would also need to check that the desired name exists in the dictionary before trying to call it, but you should be able to get the idea here.

The advantage here over reflection is that you do not have to do introspection to figure out if the method exists and is significantly faster than reflection. It also handles the issue of case (as in upper/lower case) a lot better. With reflection, GetAssets and getAssets and getassets may all be different methods (at least in C# if you want to be non-CLS compliant).

You could do reflection if you wanted to, and here is the code that you would need to do it:

private static IEnumerable CallMethodByName(string methodName, M5Record parent)
{
    var method = typeof(M5Record).GetMethod("get" + methodName,
        BindingFlags.IgnoreCase | BindingFlags.Static | BindingFlags.Public);

    if (method == null)
        method = typeof(M5Record).GetMethod("get" + methodName + "s",
            BindingFlags.IgnoreCase, BindingFlags.Static, BindingFlags.Public);

    if (method != null)
        return method.Invoke(parent, null) as IEnumerable;

    return new List();    //To keep with your pattern
}


Which handles (in a very basic way) the plural and non-plural cases for the method name being passed in, so you could pass in raw user input and it would check for both.

Code Snippets

//This method is exposed as part of the web API
public static GetAssets(string tenantName, string parentCollection, string parentId)
{
    var parent = GetParent(tenantName, parentCollection, parentId);

    if (parent == null)
        return new List<Record>();

    //...
}

private static M5Record GetParent(string tenantName, string parentCollection, string parentId) 
{
    try
    {
        return GetById(tenantName, parentCollection, parentId);
    }
    catch (InvalidOperationException)
    {
        throw new WebFaultException<ErrorResponse>(
            new ErrorResponse { Error = "Not found" }, HttpStatusCode.NotFound);
    }
}
public class MyWebAPI
{
    private static Dictionary<string, Func<M5Record, IEnumerable<Record>>> _registry;


    static MyWebAPI()
    {
        _registry = new Dictionary<string, Func<M5Record, IEnumerable<Record>>>(StringComparer.OrdinalIgnoreCase);
        _registry["asset"] = (p) => { return p.GetAssets(); };
        _registry["assets"] = _registry["asset"];
        //... etc
    }

    public static MyMethod(string methodName)
    {
        var parent = ...;
        _registry[methodName](parent);
    }
}
private static IEnumerable<Record> CallMethodByName(string methodName, M5Record parent)
{
    var method = typeof(M5Record).GetMethod("get" + methodName,
        BindingFlags.IgnoreCase | BindingFlags.Static | BindingFlags.Public);

    if (method == null)
        method = typeof(M5Record).GetMethod("get" + methodName + "s",
            BindingFlags.IgnoreCase, BindingFlags.Static, BindingFlags.Public);

    if (method != null)
        return method.Invoke(parent, null) as IEnumerable<Record>;

    return new List<Record>();    //To keep with your pattern
}

Context

StackExchange Code Review Q#146787, answer score: 6

Revisions (0)

No revisions yet.