Review of Generated Code about swinject-codegen HOT 10 CLOSED

Generated code has something like this:

func resolveInjectablePerson() -> InjectablePerson {
    return self.resolve(PersonType.self)! as! InjectablePerson
}

I think we don't need ! before as!. The generated code can be:

func resolveInjectablePerson() -> InjectablePerson {
    return self.resolve(PersonType.self) as! InjectablePerson
}

from swinject-codegen.

Generated code has something like this:

func resolveInjectablePerson(foo: AType, bar: BType, baz: String, qux: String) -> InjectablePerson {
    return self.resolve(PersonType.self, arguments: (foo, bar, baz, qux))! as! InjectablePerson
}

I think it's more understandable if the first argument is labeled like this:

func resolveInjectablePerson(foo foo: AType, bar: BType, baz: String, qux: String) -> InjectablePerson {
    return self.resolve(PersonType.self, arguments: (foo, bar, baz, qux))! as! InjectablePerson
}

Then you can use the function like this:

let p = container.resolveInjectablePerson(foo: someFoo, bar: someBar, baz: "some baz", qux: "some qux")

from swinject-codegen.

Generated code has something like this:

extension Resolvable {
    func registerInjectablePerson(registerClosure: (resolver: ResolverType) -> (InjectablePerson)) -> ServiceEntry<PersonType> {
        return (self as! Container).register(PersonType.self, factory: registerClosure)
    }
}

It uses (self as! Container), which expects the Resolvable instance must be Container type. I think it's more understandable to extend Container type for registration methods. (Resolution methods can stay in the extension of Resolvable.)

from swinject-codegen.

we can bind the current version to swinject < 2.0.0, then update the generated code, release a new version and depend on swinject >= 2.0.0.
I do not see any necessary migrations, as the code will simply be replaced.

from swinject-codegen.

@Lutzifer plz let me know when the code is ready for review.

from swinject-codegen.

This can be done, I do not have any open improvements/todos concerning the generated code atm

from swinject-codegen.

I'll try reviewing in this weekend.

from swinject-codegen.

I wrote three suggestions⬆️ What do you think about them?

from swinject-codegen.

we had an issue with (self as! Container) in an earlier version of the code, but it seems to have vanished :-)

from swinject-codegen.

@Lutzifer FYI: I'll rename Resolvable to ResolverType in Swinject 2.0.0.

Swinject/Swinject#90
Swinject/Swinject#87

CodeGen also needs migration to Swinject 2.0.0 later, but for now we can work on Swinject 1.x.

from swinject-codegen.