Issue 277 recursively resolving dependencies #280

Merged
merged 3 commits into from Jul 13, 2015

Projects

None yet

2 participants

@neoneo
Contributor
neoneo commented Jul 12, 2015

I've opted for passing the external beanfactory into Taffy's factory, and let getBean() ultimately check the external factory for the bean. If you have any remarks about the changes, please let me know.

@atuttle atuttle commented on an outdated diff Jul 13, 2015
core/factory.cfc
<cfscript>
- return structKeyExists(this.beans, arguments.beanName) or (includeTransients and transientExists(arguments.beanName));
+ return !isNull(this.externalBeanFactory) and this.externalBeanFactory.containsBean(arguments.beanName);
@atuttle
atuttle Jul 13, 2015 Owner

isNull() isn't available in CF8 and as of now we still support back that far.

@atuttle
Owner
atuttle commented Jul 13, 2015

I really like your approach of passing the external factory into Taffy's factory -- that makes a heck of a lot of sense! Just one note (see the line comment) about compatibility. This is going to be a great addition. Thanks so much!

@neoneo
Contributor
neoneo commented Jul 13, 2015

Thanks for your feedback! Do you want me to fix those isNull's? Will do that later today.

@atuttle
Owner
atuttle commented Jul 13, 2015

Yes please! Just make the appropriate changes and push to the same branch of your repo. That will automatically update this PR.

@neoneo
Contributor
neoneo commented Jul 13, 2015

I replaced the isNull calls with calls to structKeyExists (I like that better than isDefined), plus one or two minor changes (one of those is also a CF8 compat issue I believe).

@atuttle
Owner
atuttle commented Jul 13, 2015

Awesome. I'm going to go ahead and merge it in so that it can be in the bleeding edge downloads, but it won't be included in an official release until 3.1.

@atuttle atuttle merged commit 1e83bfe into atuttle:master Jul 13, 2015
@atuttle atuttle added this to the Taffy 3.1 milestone Jul 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment