Los Techies : Blogs about software and anything tech!
Chad Myers' Blog » I need some peer review on this

I need some peer review on this


So I have a problem where I have an open type:

public class ThunderdomeActionInvoker<TController, TInput, TOutput> 
    : IControllerActionInvoker
    where TController : class
    where TInput : class, new()
    where TOutput : class
{
    /*...*/
}

And I need to make a generic one of these bad-boys and then “new” it up.  The only problem is, I don’t know whether my candidate/proposed type for TInput meets the “class” and/or “new()” constraints.  There doesn’t appear to be a Type.TryMakeGenericType() method and calling MakeGenericType() blindly will toss you up a nice fat ArgumentException to catch.

I did some cursory searching, but my Google-fu has failed me this day.  Is there nothing to do this?  If not, then I scrapped something together and I wanted to see what you all thought of this just in case I’m really the first person to have needed this.  I haven’t fully unit tested this (this was a spike, so I didn’t test-drive this… I know… SHAME), so don’t just COPY AND PASTE this or bad things will happen including 7 years bad luck and maybe some rain coming in through your windows.

public static bool MeetsSpecialGenericConstraints(Type genericArgType, Type proposedSpecificType)
{
    var gpa = genericArgType.GenericParameterAttributes;
    var constraints = gpa & GenericParameterAttributes.SpecialConstraintMask;

    // No constraints, away we go!
    if (constraints == GenericParameterAttributes.None)
        return true;

    // "class" constraint and this is a value type
    if ((constraints & GenericParameterAttributes.ReferenceTypeConstraint) != 0
        && proposedSpecificType.IsValueType )
    {
        return false;
    }
           
    // "struct" constraint and this is a value type
    if ((constraints & GenericParameterAttributes.NotNullableValueTypeConstraint) != 0
        && ! proposedSpecificType.IsValueType)
    {
        return false;
    }

    // "new()" constraint and this type has no default constructor
    if ((constraints & GenericParameterAttributes.DefaultConstructorConstraint) != 0
        && proposedSpecificType.GetConstructor(Type.EmptyTypes) == null )
    {
        return false;
    }

    return true;
}

Thoughts?  Obvious bugs?

Kick It on DotNetKicks.com
Posted May 05 2009, 12:21 AM by chadmyers
Filed under: ,

Comments

chadmyers wrote re: I need some peer review on this
on 05-05-2009 12:24 AM

Between Jimmy, Matt Hinze, and R# constantly giving me crap about the ALLCAPS generic params, I finally caved into the ugly, barbarian, and heathen "TInput" motif.  I hope your happy! ;)

Rasmus Kromann-Larsen wrote re: I need some peer review on this
on 05-05-2009 4:25 AM

Hm.

I think that I would actually go down the route of considering catching the ArgumentException and probably rethrowing it in another form - although catching a "runtime exception" is bad mojo. It might mask some other underlying cause in the constructor (other ArgumentExceptions), but if you get to this point - you're failing anyway - but I'd probably mention it in the exception message :-)

What is your motivation for all this? If you did have a TryMakeGenericType method on type, you'd still get an instance, which would be the same result as the successful scenario if you just catch the ArgumentException.

If you really want to verify that it is valid -without- instantiating the type, I suppose you need your new method. But it would need thorough testing and maybe also support for the last constraint-types (derrives and interfaces?) - and it might break in C# 4.0 with co/contravariance.

AYGNI? :-)

Brian Chiasson wrote re: I need some peer review on this
on 05-05-2009 8:22 AM

The IsGenericTypeDefinition may be what you seek.  msdn.microsoft.com/.../system.type.isgenerictypedefinition.aspx

Brian Chiasson wrote re: I need some peer review on this
on 05-05-2009 8:23 AM

I forgot to mention in conjunction with the Type.GetGenericArguments method.

msdn.microsoft.com/.../system.type.getgenericarguments.aspx

Joshua Flanagan wrote re: I need some peer review on this
on 05-05-2009 8:52 AM

TOuch! TShield TMy TEyes! T:-)

Brian Chiasson wrote re: I need some peer review on this
on 05-05-2009 9:23 AM

Okay, so I didn't totally GROK the problem, but now I get it and you have sent me off on a tangent that has cost me some time this morning.

From what I can tell, based on the BCL, poking around in Reflector, and Google, you have to do the constraint checking yourself.

chadmyers wrote re: I need some peer review on this
on 05-05-2009 9:53 AM

Rasmus,

Yes, I do need it :)   Catching exceptions is bad because I'll be running this on potentially dozens or hundreds of types and that level of exception throwing/catching is unacceptable in my book.

The problem is I have an open generic type and I have candidate types to fill in the generic parameters.  This is all via reflection, so I don't have direct knowledge of the types, only their Type/reflection metadata.

I need to see if Type X will work as the "TInput" for "FooType<TInput> where TInput : class, new()"

The only way I can see this in the BCL is to try calling MakeGenericType() and hope for the best. This is unacceptable, so I'm trying to evaluate the condition before blindly calling an exception-throwing method.

Brian Chiasson wrote re: I need some peer review on this
on 05-05-2009 9:56 AM

Some more thoughts...

You or your team have to remember this method if you add another constraint on TInput.

Can TInput be generic? If TInput can be generic, will it ever be open when you attempt to create the invoker?

chadmyers wrote re: I need some peer review on this
on 05-05-2009 1:18 PM

@Brian:

At least in this specific case, I've already checked to make sure that TInput is not an open generic type  (i.e. Foo<String> is ok, but Foo<T> is not).

If we add another constraint to TInput (such as replacing 'class' with 'struct'), this code I posted should handle it fine.

David Nelson wrote re: I need some peer review on this
on 05-05-2009 3:02 PM

"Catching exceptions is bad because I'll be running this on potentially dozens or hundreds of types and that level of exception throwing/catching is unacceptable in my book."

I am curious as to why this is automatically unacceptable. Is it because of the potential performance problem? Because I am not sure that doing extensive reflection on the types is going to be any better. And I certainly wouldn't go down the much more complicated way until I had at least tried the simple way to see if it feel within my requirements.

chadmyers wrote re: I need some peer review on this
on 05-05-2009 4:50 PM

@David:

I'm looping over the types anyhow, no need to make the situation worse by adding a bunch of exceptions to the mix.

Exceptions are, by definition, exceptional. This is not an exceptional case. These are normal types that may or may not  meet certain criteria I need.

Throwing and catching a lot of exceptions is just bad coding unless there is no other alternative.

Chris Tavares wrote re: I need some peer review on this
on 05-06-2009 2:33 AM

Stupid question department, probably, but my guess is the ActionInvoker class is doing a ton of reflection. If it's doing reflection anyway, why is it generic at all? Are you actually saving anything performance wise or gaining anything by using the generic type arguments?

chadmyers wrote re: I need some peer review on this
on 05-06-2009 2:48 AM

@Chris: I'm looping through and discovering all the "Controller" types in a given assembly, then finding any candidate methods that would be "Actions",  and then configuring a ThunderdomeActionInvoker (heretofore TAI) in StructureMap to invoke those actions.

I do the nasty reflection business at startup and then configure generic, properly-typed invokers and things to avoid the reflection hit every single request to every single action.

Brian Chiasson wrote re: I need some peer review on this
on 05-06-2009 9:23 AM

@Chad

I was thinking more along the lines of an Interface definition or base class, versus the "typical" (lack of better word) constraints.

I am guessing that you already considered taking the "reflection hit" the first time the request is made to the specific TAI as opposed to loading them all at once?

David Nelson wrote re: I need some peer review on this
on 05-08-2009 12:14 PM

"Throwing and catching a lot of exceptions is just bad coding unless there is no other alternative."

In principle I agree. But in this case, it isn't your code that is throwing the exceptions; you are simply being forced into the position of catching them by someone else's bad design. Refusing to call code that you know will throw an exception simply because you don't consider the circumstance exceptional - when the only alternative is complex, difficult to maintain, and very unlikely to be 100% effective - doesn't make much sense to me.

Copyright Los Techies 2008, 2009. All rights reserved.
Powered by Community Server (Commercial Edition), by Telligent Systems