dodrugs icon indicating copy to clipboard operation
dodrugs copied to clipboard

High-level feedback

Open jcward opened this issue 8 years ago • 1 comments

Hi @jasononeil -- so far I've only looked briefly at the readme (e.g. curb appeal, as a prospective user might), so here're some initial thoughts from that.

I have lots of questions wrt scope. Where does the var appInjector = ... declaration go? Then what is the scope of those definitions? Do they apply to all classes? I see the "myapp" namespace string. But I don't see that string elsewhere else (e.g. on the constructors)... I see how you use the appInjector instance on manual injection, so I suppose that injector is passed to clients who want to use it (or made a static singleton :smiling_imp: :see_no_evil:). But which injectors apply to which constructors? For constructors, does macro magic make the injectable parameters optional? So I can still call it with other or custom parameters.

Regarding the Concepts section:

  • Section 1 -- I don't quite get the gist here -- I'm sure I will as I start using the library. However, the sentence anytime you have an Injector<"app">... sounds especially important, so I wish I grok'ed it. I don't see an Injector<"app"> anywhere above in the sample code.
  • Section 2 -- I haven't used other libs, but this lib sounds good, especially the sentence everything available immediately - async is often tedious.
  • Section 3 & 4 -- sounds good.

Hope this "quick review by a dummy" is helpful. :smile: Again, I'm sure this'll all make more sense once I dive in.

Cheers! -Jeff

jcward avatar May 03 '17 15:05 jcward

Hi Jeff - this is really useful feedback. I really appreciate you taking the time to read through it and try to share what is confusing or misleading or what-not, as that is the things it's way too easy to be familiar with and not explain properly when it's something you've spent a lot of time with yourself.

I think I'll have to have a go at clarifying a bunch of things in the README. I'll leave the issue open until I get a chance to rework it and make things more clear.

jasononeil avatar May 07 '17 12:05 jasononeil