hmlongco / Resolver

Swift Ultralight Dependency Injection / Service Locator framework
MIT License
2.14k stars 188 forks source link

Injection with any number of arguments (Params). #31

Closed ahmadmssm closed 4 years ago

ahmadmssm commented 4 years ago
  1. Added Injection with any number of arguments (Params).
  2. Added Unit test fo the feature.
  3. Added documentation.
ahmadmssm commented 4 years ago

@hmlongco

ahmadmssm commented 4 years ago

@hmlongco Do you see that it is a good idea to create a class for every group of params, it is very uncomfortable, plus we are already working with it and my colleagues see that passing arguments as a dictionary then getting them by key is not the best way to go, you can see what is Koin doing.

hmlongco commented 4 years ago

This is what I meant by making a separate argument collection class....

class Args {
    private var args: [Any?] = []
    init<T0,T1>(arg0:T0? = nil, arg1:T1? = nil) {
        args.append(arg0)
        args.append(arg1)
        // extend as needed
    }
    static func arg0<T>(_ args: Any?) -> T? {
        guard let args = args as? Args else { return nil }
        return args.args[0] as? T
    }
    static func arg1<T>(_ args: Any?) -> T? {
        guard let args = args as? Args else { return nil }
        return args.args[1] as? T
    }
}

class Dummy {
    init(arg0: String, arg1: Int) { }
}

extension Resolver {
    static func myRegistrations() {
        register { _, a in Dummy(arg0: Args.arg0(a)!, arg1: Args.arg1(a)!) }
    }
    static func test() {
        let d: Dummy = Resolver.resolve(args: Args(arg0: "This is a test", arg1: 24))
    }
}

It does exactly the same thing and doesn't need to be embedded in Resolver.

That said, I still don't like the general solution in either case. It basically erases the type information, most of the time it requires explicit unwrapping of the resulting arguments, and positional "generic" arguments are highly prone to failure should the object initializer's type signature change.

And all of which goes against my viewpoint that injection exists to build the object class hierarchy, NOT to pass data. If an object requires configuration I'd do something like the following....

class DummyViewModel {
    func configure(name: String, mode: Int) { }
}

class DummyViewController: UIViewController {

    var name: String!
    var mode: Int = 0

    @Injected var viewModel: DummyViewModel

    override func viewDidLoad() {
        super.viewDidLoad()
        someSetup()
        viewModel.configure(name: name, mode: mode)
    }
}

Type information is preserved. Argument order is maintained. Nor is forced unwrapping of arguments required.

Plus I now have the added benefit of being able to control exactly when and where in the code I call my configuration function (load function, etc.) and consequently I can ensure that any needed initialization is performed prior to doing so.

ahmadmssm commented 4 years ago

@hmlongco Thanks for the clarification, there is a concept called Assisted Injection which is passing parameters to the constructor of the injected objects, so although it may be prone to failure as you mentioned, but it is also important to the context of a DI framework and maybe someone to choose another library which is more complicated when it comes to initial setup and less elegant than Resolver but it has this feature, so let me disagree with you, Injection exists to build the object class hierarchy AND to pass data with the injected objects, and a good example of this is Google Guice, Dagger2, Koin and even Swinject, all of those are DI libraries. Thank you..

hmlongco commented 4 years ago

Three of the four libraries you just mentioned are for Android, so I'm not too worried about someone switching to them... ;)

I'm considering your argument, and I have to admit that my primary reservation is simply not wanting to bulk up Resolver with extra code, especially when, as shown above, it's fairly easy to create outside of Resolver itself.

For the moment, Resolver is fairly opinionated around the idea of building the object class hierarchy first, then configuring it with data second. I wrote about this, and about my dislike for constructor injection, in Modern Dependency Injection is Swift.

That's pretty much a requirement with ViewControllers, for example, and even more so if you move to using Injected wrappers, where passing args as part of the initialization phase simply isn't possible.

@Injected var viewModel: MyViewModel

I might also mention that Apple itself is moving in that direction with SwiftUI and EnvironmentObjects.

@EnvironmentObject var viewModel: MyViewModel

Where again you get the object first via the property wrapper, and then do any needed parameter passing/configuration in a second phase.

At any rate, I appreciate the thoughts.

ahmadmssm commented 4 years ago

@hmlongco I code both Android and iOS :D and I knew that you may mention this that is why I added Swinject. Swift UI is still growing so we do not know if Apple is going to support this in the future or not. Anyway, we ended up including the class instead of using it as a pod and we have added a utility class for arguments resolving. Thank you very much and keep the good work ;)

hmlongco commented 4 years ago

Okay, two more thoughts....

One is that by keeping Resolver small and lightweight and basically just a single file I'm trying to encourage people to simply include it directly into their projects and use it/mutate it as needed. As you did.

Second, and in regard to your SwiftUI comment... I think you can assume that Apple will continue to "support" it. In fact, and much like Objective-C, it's UIKit's days that are now numbered...

Thanks, and have a good Sunday.

hmlongco commented 4 years ago

So... I was bored and decided to clean this up a bit.

Given a variant of my original argument class...

struct ResolverArgs {
    private var args: [Any?] = []
    init(_ args: Any?) {
        guard let args = args as? ResolverArgs else {
            fatalError("Argument type not ResolverArgs")
        }
        self.args = args.args
    }
    init<T0>(arg0:T0) {
        args.append(arg0)
    }
    init<T0,T1>(arg0:T0, arg1:T1) {
        args.append(arg0)
        args.append(arg1)
    }
    func get<T>(_ index: Int) -> T {
        return args[index] as! T
    }
}

And a class like...

class XYZEditService {
    let editing: Bool
    let name: String
    init(editing: Bool, name: String) {
        self.editing = editing
        self.name = name
    }
}

You could do...

        resolver.register { (r, a) -> XYZEditService in
            let a = ResolverArgs(a)
            return XYZEditService(editing: a.get(0), name: a.get(1) )
        }

Resolving it as...

let service: XYZEditService? = resolver.optional(args: ResolverArgs(arg0: true, arg1: "Fred"))

So multiple, typed arguments, when needed, and built on a struct-based entity and all using the existing argument mechanism.

What do you think?

One thing I still like about this is that it doesn't break the existing mechanism and it can also be used in the .properties mutator.

ahmadmssm commented 4 years ago

OK, I took this one more step so the user has the option to pass 5 args or more, after adding this change everything will like as it is without the need for any breaking change ar anything that may break the existing mechanism.

class ResolverArgs {

    private var argsArray: [Any?] = []

    init(args: Any...) {
        self.argsArray = args.compactMap { ($0 as? [Any])?.first }
    }

    init<T0, T1, T2, T3, T4, T5>(arg0: T0? = nil,
                                 arg1: T1? = nil,
                                 arg2: T2? = nil,
                                 arg3: T3? = nil,
                                 arg4: T4? = nil,
                                 arg5: T5? = nil) {
        argsArray.append(arg0)
        argsArray.append(arg1)
        argsArray.append(arg2)
        argsArray.append(arg3)
        argsArray.append(arg4)
    }

    static func arg0<T>(_ args: Any?) -> T? {
        return resolveArgFor(index: 0, args)
    }

    static func arg1<T>(_ args: Any?) -> T? {
        return resolveArgFor(index: 1, args)
    }

    static func arg2<T>(_ args: Any?) -> T? {
        return resolveArgFor(index: 2, args)
    }

    static func arg3<T>(_ args: Any?) -> T? {
        return resolveArgFor(index: 3, args)
    }

    static func arg4<T>(_ args: Any?) -> T? {
        return resolveArgFor(index: 4, args)
    }

    static func arg5<T>(_ args: Any?) -> T? {
        return resolveArgFor(index: 5, args)
    }

    private static func resolveArgFor<T>(index: Int, _ args: Any?) -> T? {
        if let resolverArgs = args as? ResolverArgs {
            if let arg = resolverArgs.argsArray[exist: index] as? T {
                return arg
            }
            fatalError("Wrong Argument index")
        }
        else {
            return args as? T
        }
    }

    static func argument<T>(from args: Any, argumentNo: Int) -> T? {
        if let args: T = resolveArgFor(index: argumentNo, args) {
            return args
        }
        return args as? T
    }
}

Then I created this extension.

// MultiParams
extension Resolver {

    public func optional<Service>(_ type: Service.Type = Service.self,
                                         name: String? = nil,
                                         arg0: Any? = nil,
                                         arg1: Any? = nil,
                                         arg2: Any? = nil,
                                         arg3: Any? = nil,
                                         arg4: Any? = nil,
                                         arg5: Any? = nil) -> Service? {
        let args = ResolverArgs(arg0: arg0, arg1: arg1, arg2: arg2, arg3: arg3, arg4: arg4, arg5: arg5)
        return optional(type, name: name, args: args)
    }

    public final func optional<Service>(_ type: Service.Type = Service.self,
                                        name: String? = nil,
                                        arguments: Any...) -> Service? {
        let args = ResolverArgs(args: arguments)
        return optional(type, name: name, args: args)
    }

    public func resolve<Service>(_ type: Service.Type = Service.self,
                                  name: String? = nil,
                                  arg0: Any? = nil,
                                  arg1: Any? = nil,
                                  arg2: Any? = nil,
                                  arg3: Any? = nil,
                                  arg4: Any? = nil,
                                  arg5: Any? = nil) -> Service {
        let args = ResolverArgs(arg0: arg0, arg1: arg1, arg2: arg2, arg3: arg3, arg4: arg4, arg5: arg5)
        return resolve(type, name: name, args: args)
     }

    public func resolve<Service>(_ type: Service.Type = Service.self,
                                 name: String? = nil,
                                 arguments: Any...) -> Service {
        let args = ResolverArgs(args: arguments)
        return resolve(type, name: name, args: args)
    }

    func arg0<T>(from args: Any) -> T? {
        return ResolverArgs.arg0(args)
    }

    func arg1<T>(from args: Any) -> T? {
        return ResolverArgs.arg1(args)
    }

    func arg2<T>(from args: Any) -> T? {
        return ResolverArgs.arg2(args)
    }

    func arg3<T>(from args: Any) -> T? {
        return ResolverArgs.arg1(args)
    }

    func arg4<T>(from args: Any) -> T? {
        return ResolverArgs.arg4(args)
    }

    func arg5<T>(from args: Any) -> T? {
        return ResolverArgs.arg5(args)
    }

    func argument<T>(from args: Any, argumentNo: Int) -> T? {
        return ResolverArgs.argument(from: args, argumentNo: argumentNo)
    }
}

And add this extension to avoid index out of range

extension Collection where Indices.Iterator.Element == Index {
    subscript (exist index: Index) -> Iterator.Element? {
        return indices.contains(index) ? self[index] : nil
    }
}

Then we can inject it like this

 register { (resolver, args: Any) -> MoviesPage in
            // let page = args as! Int 
            // OR
            // let page: Int = resolver.argument(from: args, argumentNo: 0)!
            // OR
            let page: Int = resolver.arg0(from: args)!
            return getReactiveAPI(api: TrendingMoviesAPI(page: page))
        }

And use it like this

//       let trendingMoviesAPI: MoviesPage = resolver.resolve(args: page)
// OR
//       let trendingMoviesAPI: MoviesPage = resolver.resolve(arg0: page)
// OR
        let trendingMoviesAPI: MoviesPage = resolver.resolve(arguments: page)
hmlongco commented 4 years ago

So.... I've thought about it and I think I've decided to implement a solution based on ResolverArgs, but in a way that's a little different than we've discussed.

Basically I think I'm going to add another "args" parameter that's available as a registration factory variant. If you "skip" the existing args slot and import a new third parameter you're going to get a ResolverArgs collection type, with passed arguments directly accessible as subscripts.

        resolver.register { (_, _, args) -> XYZEditService in
            XYZEditService(editing: args[0]!, name: args[1]!)
        }

The new arguments are passed using the resolution syntax we've seen before.

        let service: XYZEditService = resolver.resolve(arg0: true, arg1: "Fred")

Any existing argument that might have been passed in old code is still available in the original args parameter, but in in addition is automatically promoted to args[0] in the new parameter.

        // old way
        resolver.register { (_, arg) -> XYZEditService in
            let mode: Int = arg as! Int
            SomeService(mode)
        }
        // new way
        resolver.register { (_, _, args) -> XYZEditService in
            SomeService(args[0]!)
        }

As to the original PR solution, I didn't really like adding a boatload of argX functions to the Resolver class and static class definitions. Also, my way of obtaining a typed parameter is a simple subscript to the provided args parameter that doesn't requiring passing the arguments in and out of an accessor function each and every time, and ResolverArgs is a lightweight struct as opposed to a heavier class object.

If you don't want to upgrade all of your current code base but want to work with the new implementation you could simply pass the new ResolverArgs structure to an upgraded version of your code that simply pulled the parameter from args.

    func firstArgument<T>(from args: Any) -> T? {
        if let args = ResolverArgs, let arg = args[0] as? T {
            return arg
        }
        return nil
    }

The updated code is currently in the develop branch if you want to play with it.

The only thing I'm still on the fence about is whether or not the extra parameter signature is a good idea, or if multiple parameters are really needed that badly then simply biting the bullet and just changing the current args parameter from Any? to ResolverArgs and be done with it. That cuts the code complexity quite a bit.

I just don't have a good feel for how many people are using the existing implementation.

ahmadmssm commented 4 years ago

Actually I like the idea of the third parameter and I agree with you that adding a boatload of argX functions to the Resolver class and static class definitions is not a good idea, but it was the simplest, fastest and the cleanest way to achieve what i want, So, yes I like the new approach. The only thing I see that can be better is the ability to get the arguments using this syntax arg0(), arg1(), ... instead of args[index] because the first one is more cleaner and the second one is more error-prone. So I would say, go ahead with the new implementation, It will be a major improvement for your library. Thank you very much for considering my suggestion and solution.

hmlongco commented 4 years ago

I expanded on the above a bit more here: https://github.com/hmlongco/Resolver/issues/44