search

tomkuijsten / restup

Webserver for universal windows platform (UWP) apps
MIT License
114 stars 48 forks source link

Passing implementation instead of interface as constructor argument #130

Closed thijscrombeen closed 7 years ago

thijscrombeen commented 7 years ago

I have a controller which takes a few interfaces as constructor arguments. When I register this controller I pass in implementations as arguments.

This is however not allowed due to code in the ReflectionHelper class. Would you accept a pull request where it also checks the IsAssignableFrom?

thijscrombeen commented 7 years ago

Is my question perhaps not clear enough? Or is Restup no longer actively maintained?

Thanks

dm-CaT commented 7 years ago

Are you sure it is possible? As I know there is a limitation of UWP which doesn't allow to declare public classes (eg. controllers) with interface as a type of constructor parameter.

Jark commented 7 years ago

Hi @thijscrombeen,

Sorry for the late reply!

I don't actively work on restup any more, but I do try and maintain it and fix critical bugs. The reason for this is that it's looking like asp .net core is being supported officially pretty soon (see http://www.devvy.nl/?p=193 to get it to work now).

However I'm happy to accept a pull request that uses IsAssignableFrom, as long as it contains a unit test proving it works.

@dm-CaT you might be able to work around this by using a class library instead of a windows runtime component. That might work fine

And I'll promise to reply / act on things a bit faster in the future.

Thanks,

Jark

thijscrombeen commented 7 years ago

I have created a pull request #131

Thanks for replying

Edit: It seems unable to build on appveyor. is there anything i can do about that?

Jark commented 7 years ago

Hi @thijscrombeen,

Thanks for your contribution! Just had a quick look and looks good from a glance, will run it up in my lunch break and give it a closer look.

As for the appveyor build, it doesn't look like your change would've broken that so I think it's something to do with uwp support in appveyor, it's been a while since we last ran it, so it's possible tom might have to make a few changes. I'll run the tests locally here to verify your change works and then we can merge it back.

Cheers,

Jark

Jark commented 7 years ago

Hi @thijscrombeen,

Just had a look and ran the tests, all looks good. Thanks for your contribution!

Cheers,

Jark

Closed #131