Remove the need for <T> in converters. about umbraco-ditto HOT 11 CLOSED

I'm happy with removing the <T> constraint and using properties from the PropertyDescriptor - as you say, it makes sense!

re: Breaking changes... since we're currently following SemVer, we're still on v0.x - so completely within the realm of breaking changes. I know it is a pain in the arse for some devs to deal with these scenarios, but truth is that Ditto is still under active development (pre-release) - so until a v1.0 is released we have freedom to change APIs.

from umbraco-ditto.

👍

from umbraco-ditto.

Wizzzard! I was hoping you'd say that. 😄

from umbraco-ditto.

I need to get my head back round these as I haven't looked for a while. A couple of comments off the top of my head though.

By supporting generics, we do make this a bit easier in some places, such as using Enumerable.Empty< T>() to return an empty collection of type T, which you couldn't do if we didn't have a generic type. Also, with < T> we are requiring that T is a class, which is a requirement for the type converter to work (i think) so removing the generic type means you'll have to perform any relevant checks manually (if there are any).

Like I say, I'm not sure if any of these are a problem, I just looked through the code at where generics is being used in the current code, and how removing this will affect those.

I think the main original requirement for this was because .As< T> was the only way to use ditto originally, however with the addition of the .As(Type type) method added, it may not be as big a requirement.

from umbraco-ditto.

Those are fair points.

Enumerable.Empty is the trickiest of those to deal with, That can be invoked though, I think I need to make an EnumerableInvocator helper class somewhere to store the cached methods since I would have three then. Empty, Cast, and FirstOrDefault.

For the constraints... Type.IsValueType and Type.IsClass should cover that.

from umbraco-ditto.

Hi @mattbrailsford, perhaps I misunderstand, but can't an Emtpy collection of 'myType' be returned with:

return Enumerable.Empty<object>().Cast(myType);

where Cast(Type type) is an extension:

public static IEnumerable Cast(this IEnumerable iEnumerable, Type type)
{
    MethodInfo methodInfo = typeof(Enumerable).GetMethod("Cast");
    MethodInfo genericMethodInfo = methodInfo.MakeGenericMethod(type);

    return genericMethodInfo.Invoke(null, new[] { iEnumerable }) as IEnumerable;
}

from umbraco-ditto.

Probably. But now we need a custom Cast method, and IMO the result is less readable.

from umbraco-ditto.

arh, what @JimBobSquarePants has just said

@mattbrailsford but the end result is simpler syntax for the end consumer

from umbraco-ditto.

@Hendy fair point.

from umbraco-ditto.

We already have the Cast and FirstOrDefault ones. I'm happy to make the changes and I'll keep it as simple as possible.

from umbraco-ditto.

Closing this discussion thread. We can continue (if needed) on PR #55.

from umbraco-ditto.