bilal-fazlani / commanddotnet

A modern framework for building modern CLI apps
https://commanddotnet.bilal-fazlani.com
MIT License
560 stars 32 forks source link

Dependency Injection is poorly documented and misleading #445

Closed MiniaczQ closed 2 years ago

MiniaczQ commented 2 years ago

I am aware that this project is not being worked on anymore, but this may help others falling for this.

I've been trying to add dependency injection to my project, to mimic the nature of ASP.NET.

What I expected was that anything that fails to be resolved through this library (or perhaps annoted, as additional control measure) would be resolved by a DI resolver of user's choice.

What actually happens is, the library provides support for resolving only 2 types of arguments, IArgumentModel and CommandClass (what even is this? it doesn't exist in the code base).

The problem with this is that those are the opposite of what a DI should provide, this is what the library is responsible for, objects that originate from arguments.

It took a while, but I found that what I'd actually need to do for this to work is to register every service with the parameter resolver. Now, this is extremelly inconvenient and poorly scales with new services.

While I'm still not 100% convinced this is the right conclusion, I've spent way too much time trying to figure this out hence the 'poor documentation' part.

drewburlingame commented 2 years ago

@MiniaczQ the project is still being maintained, but new features are not currently being added by the maintainers. It sounds like you're frustrated and using the issue system to express your frustration rather than using the discussions system to problem solve like others.

Based on your explanation, it seems there is a misunderstanding of how CommandDotNet integrates with DI containers and how the DI container. I have many console projects using various DI containers and I don't need to register the services with a parameter resolver. Based on discussions, others are using DI containers successfully. That leads me to believe there isn't a problem with the DI integrations.

What I expected was that anything that fails to be resolved through this library (or perhaps annoted, as additional control measure) would be resolved by a DI resolver of user's choice.

It's the opposite order. Any registered DI resolver is used first. As I reread the docs, this seems fairly clear to me.

When a resolver is registered, it will be used to resolve instances for

  • command classes using IDependencyResolver.Resolve
  • IArgumentModel classes using IDependencyResolver.TryResolve.

DI containers have different behaviors for Resolve and TryResolve. In most cases, Resolve will throw an exception if an instance isn't registered. Some containers, like MicrosoftDependencyInjection, will return null instead. When TryResolve is used or Resolve returns null, CommandDotNet will attempt to instantiate an instance.

What would be constructive is to find out what you think is misleading and how the docs could be updated to make it more clear for you. Would it help if the last line was updated to CommandDotNet will attempt to instantiate an instance if no instance is returned?

A comment you made that's confusing to me and makes me think there's a misunderstanding of how DI integrates is

The problem with this is that those are the opposite of what a DI should provide, this is what the library is responsible for, objects that originate from arguments.

Services that would be resolved by a DI container are separate from parameters that are parsed from the arguments. They follow two different code paths.

If you'd like help with this, start a discussion and explain clearly what you're trying to achieve, how it's failing, which DI container you're using and what you've done to troubleshoot so far. Code will probably be very helpful. If you can link to a repo we can review, that will be helpful, otherwise, post samples of your DI registration and the command, arg model, etc that aren't working as you expected.

I'll close this issue and we can investigate in the discussion.

MiniaczQ commented 2 years ago

Thank You for the quick reply. You're right. Sorry about the lash out. Wasted a few hours just to scrap the DI layer in the end. I'll make a discussion for it later today. As for code, I'll try to mock something up. Your explaination seems to align with my expectations from reading the documentation, I guess the problem was with the code not delivering that.