So, are we eliminating non-generic code from CSLA 4.0?

rxelizondo posted on Wednesday, December 16, 2009

Rocky, I understand that you went ahead and eliminated non-generic data portal calls. Have you decided if you are going to eliminate other non-generic code once and for all?

Just for fun, one more reason why non-generic methods SHOULD be eliminated:

public static PropertyInfo<int> XyzProperty = RegisterProperty<int>(m => m.Xyz);
public void DoIt()
{
SetProperty(XyzProperty, "Hey I am a string, when I should be an int, watch me crash at runtime!!!");
}

Yes, I know, it’s another one of my silly pointless posts but come on, there is zero need to let something like this happen in this day in age when issues like this can be nonexistent…. At least that’s what it looks to me, please help me undestand if am I missing something.

rxelizondo replied on Thursday, December 17, 2009

Just a quick observation, I think the two methods listed below (generic and non generic equivalents) should return the same value but they don’t.  

Perhaps there is a good reason they are implemented the way they are but I just figure I bring them up to your attention (Csla.Core.BusinessBase).

protected P ReadProperty<P>(PropertyInfo<P> propertyInfo)
protected object ReadProperty(IPropertyInfo propertyInfo)

You can tell right away what’s the issue by just looking at the code for the methods but I just created a little test app just for clarity.

private static PropertyInfo<int> XyzProperty = RegisterProperty<int>(m => m.Xyz);
public void DoIt()
{
    var nonGen = ReadProperty((Csla.Core.IPropertyInfo)XyzProperty);
    var yesGen = ReadProperty<int>(XyzProperty);

    Debug.Assert(object.Equals(nonGen, yesGen), "Why did we get here??");
}

ajj3085 replied on Thursday, December 17, 2009

Hmm, I wonder if removing the non-generic overload would limit extensibility around IPropertyInfo.

rxelizondo replied on Thursday, December 17, 2009

Note that regarding the ReadProperty post… for that particular post, I didn’t mean to say that the non generic method should be removed but more than in my mind (and for consistency), the return values should be the same for both of them (other than one has the potential of returning a boxed value type). I didn’t really take the time to dig in on how this methods are used by the framework.

I am perfectly aware that there are times when a non generic interface is needed to account for the lack of generic covariance and contra-variance support on the current versions of the framework, so I am ok with the non-generic support.

If each ReadProperty supposed to work the way it does then I think it would be more appropriate to rename the method just to avoid the confusion or if it’s meant to only be used by the framework then perhaps change the scope to internal.

rfcdejong replied on Sunday, December 20, 2009

We are using some reflection for dynamic business properties which require the non-generic IPropertyInfo.

RockfordLhotka replied on Sunday, December 20, 2009

CSLA itself uses the non-generic IPropertyInfo in some cases. Generics are not polymorphic, so it is important to have some generic base type or interface for nearly all generic types.