revoframework / Revo

Event Sourcing, CQRS and DDD framework for C#/.NET Core.
https://docs.revoframework.net/
MIT License
644 stars 70 forks source link

Throw an exception if generic commands are used #37

Closed RustamGulamov closed 3 years ago

RustamGulamov commented 3 years ago

I created command image

But if I send generic command (for example CreateReport<string> ) by commandBus, an exception is thrown.

Cannot be commands generic ?

RustamGulamov commented 3 years ago

@martinzima

martinzima commented 3 years ago

It seems that generic commands and handlers aren't picked up by the framework during the type discovery. This might be easy to fix, will probably do it later this week for the next version.

martinzima commented 3 years ago

@RustamGulamov As it turns out, it's not that simple (if even possible) to register open generic command handlers in Ninject. I fixed a few bugs in the pull request, so that you can use generic commands, but you still will have to register the command handler in a NinjectModule manually yourself like this:

this.BindCommandHandler<ReportCommandHandler<string>>();

jeffward01 commented 1 year ago

@RustamGulamov As it turns out, it's not that simple (if even possible) to register open generic command handlers in Ninject. I fixed a few bugs in the pull request, so that you can use generic commands, but you still will have to register the command handler in a NinjectModule manually yourself like this:

this.BindCommandHandler<ReportCommandHandler<string>>();

I have not looked at the code, but perhaps can you apply something like Scrutor and make the scanning easier? Some consider it 'code-smell', if you have to manually do something each and every time (it's very easy to forget a registration and make a mistake).

I am just curious - why Ninject?

Ninject has some of the worst performance among all popular IOC containers in the C# world. I'm genuinely curious why its your choice, perhaps I am missing something.

martinzima commented 1 year ago

@jeffward01 The manual registration is just a workaround for using generic commands as generic command handlers are something currently not supported in Revo and not something I would generally recommend (and I am still not even convinced that generic commands are a good idea in general, which is why this has not been implemented yet).

As for the Ninject thing - it's here mostly for legacy reasons (back when I started developing Revo, Ninject was still an actively maintained project). It's been on my to-do list for a long time, I aim to replace it sometime in the future (most likely with the official Microsoft DI extensions) - but mostly just to get rid of the dependency, not because of performance issues or because it would be lacking in features. For a majority of Revo applications, I am willing to bet that the performance improvement if we switched to a different DI will be negligible and the real bottlenecks will be elsewhere (e.g. EF Core, SQL database, etc.).