tags:

views:

210

answers:

4

Just found a bit of code someone here had written to access some DB Entities...

public static OurCustomObject GetOurCustomObject(int primaryKey)
{
    return GetOurCustomObject<int>(primaryKey, "usp_GetOurCustomObjectByID");
}

public static OurCustomObject GetOurCustomObject(Guid uniqueIdent)
{
    return GetOurCustomObject<Guid>(uniqueIdent, "usp_GetOurCustomObjectByGUID");
}

private static OurCustomObject<T>(T identifier, string sproc)
{

    if((T != typeof(int)) && (T == typeof(Guid)))
    {
        throw new ArgumentException("Identifier must be a string or an int");
    }

    //ADO.NET Code to make DB Call with supplied sproc.
}

Theres just something about it that doesn't seem very generic. The fact that the sprocs are passed into the inner method feels ugly. but the only way I can see around that is to have an if/else in the private method along the lines of

if(type == int)
    sproc = "GetByID";
else if (type == Guid)
    sproc = "GetByGUID";

Also the exception throwing looks ugly as well... is there anyway to use a where T : clause

e.g.

private static OurCustomObject<T>(T identifier) where T : int OR Guid

Any suggestions on how to clean this up a little.

+8  A: 

You can't specify a constraint which says "it's one of these two", no.

What you could do is:

Dictionary<Type, string> StoredProcedureByType = new Dictionary<Type, string>
{
    { typeof(Guid), "GetByGUID" },
    { typeof(int), "GetByID" }
};

Then use:

string sproc;
if (!StoredProcedureByType.TryGetValue(typeof(T), out sproc))
{
    throw new ArgumentException("Invalid type: " + typeof(T).Name);
}

This is probably overkill for just a couple of types, but it scales well if you have a lot of types involved.

Given that both types are value types, you can make it a bit more robust with a constraint of:

where T : struct

but that will still allow byte etc.

Jon Skeet
Cheers Jon .
Eoin Campbell
A: 

There is no support for such a where clause to limit the type parameter in that way (unfortunatelly).

In this case, you might find that passing identifier as an object, and not using generics is actually cleaner (when I needed something similar, that is what I ended up doing: seemed to be the least worst approach).

This is an area that C# is weak, being neither a dynamic language or being able to specialise the template (C++ like, only providing implementations for T = int and T = Guid).

Adendum: In this case I would likely stick with the overload, but change the type check to an Assert as that is a private helper method.

Richard
Generic types are not templates, and are not intended to be. One could make the argument that C++ templates are "weak" because, unlike generic types, they cannot be constructed with a new type argument after the template has been compiled into a library. But that would be a silly argument; the simple fact is that generics and templates, though superficially similar textually, actually have very different semantics and solve different problems.
Eric Lippert
@Eric: True, but that does not stop there being times when a specialisation would really help.
Richard
+2  A: 

The code you provided looks reasonably fine to me, because private static OurCustomObject<T>(T identifier, string sproc) is private. I would even drop the exception checking from this method, because again - its private, so this class controls what gets passed to the method. The if statement would be rather horrible and over-engineered.

Grzenio
+1  A: 

The neatest thing to do is probably doing something like this:

public interface IPrimaryKey 
{
}

public class PrimaryGuidKey(Guid key) : IPrimaryKey 

public class PrimaryIntegerKey(int key) : IPrimaryKey

private static OurCustomObject<T>(T identifier) where T : IPrimaryKey
the_ajp