[Suggestion] Custom Registration Library - Making param arrays compatible with custom type conversions

[Bogey]

Hi,

after toying around with the Custom Registration project a bit more, I've found another issue that might be interesting to fix in future versions. Posting the issue & a possible solution here to maybe help addressing this in the future.

So far, the library supports registration of param arrays. However in its current state, that is not compatible with custom input parameter conversions to my understanding. E.g.

public void test_works(params int[] basicType)
{ /* ... */ }

works, but

public void test_does_not_work(params MyClass[] customType)
{ /* ... */ }

would not work, even if you defined a conversion for MyClass. This is because the library seems to use its own TypeConversion class to do any type conversions for parameter arrays (ParamsRegistration.cs):

var testParam = Expression.IfThen(Expression.Not(Expression.TypeIs(paramExpr, typeof(ExcelMissing))), Expression.Call(argsListVarExpr, "Add", null,
TypeConversion.GetConversion(paramExpr, argsType)));


The following relatively simple modifications did the trick for me, though, and uses custom conversions instead. First of all, call ProcessParamsRegistrations BEFORE registering your custom conversions:

Registration.GetExcelFunctions()
                        .ProcessParamsRegistrations()
                        .ProcessParameterConversions(conversionConfig)
                        .ProcessAsyncRegistrations(nativeAsyncIfAvailable: false)
                        .ProcessFunctionExecutionHandlers(functionHandlerConfig)
                        .RegisterFunctions();


Also, the following substitutions must be made in ParamsRegistration.cs (just pasting the changes here as I've done more modifications to that file, hence don't want to upload the whole .cs to avoid confusion):


// Old:
// var paramExpr = Expression.Parameter(typeof(object), "arg" + i);

// Modified:
var paramExpr = Expression.Parameter(argsType, "arg" + i);


-- and --

// Old:
// var testParam = Expression.IfThen(Expression.Not(Expression.TypeIs(paramExpr, typeof(ExcelMissing))), Expression.Call(argsListVarExpr, "Add", null, TypeConversion.GetConversion(paramExpr, argsType)));


// Modified:
var testParam = Expression.IfThen(Expression.Not(Expression.TypeIs(paramExpr, typeof(ExcelMissing))), Expression.Call(argsListVarExpr, "Add", null, paramExpr));


-- and --

// Old:
// allParamTypes.Add(typeof(object));

// Modified:
allParamTypes.Add(argsType);



Can't guarantee this doesn't create any other issues at some point, however so far this seems to work fine for me.

[Govert van Drimmelen]

Hi Bogey,

Thank you very much for this (and your previous) feedback on the CustomRegistration.

I've found myself a bit confused by my (maybe ambitious) goal of allowing multiple type and other conversions, and have found that the interaction becomes complicated.

I know some combinations of features don't work together at the moment, though I'm not yet sure why not.

If you would like to help on the project, I'd be happy to register you as a collaborator on GitHub.

Maybe we can then think of how to add a bit more testing, and formalize how the conversions should be combined. I'm sure tools like PostSharp have run into the same issues.

Anyway, I am keeping track of your comments and suggestions and will apply them when I get a chance to dig into the project again.

Cheers,

Govert

[Govert van Drimmelen]

I've added this as an issue on GitHub: https://github.com/Excel-DNA/Registration/issues/5

-Govert