Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement post instanciation method call on service #20

Merged
merged 8 commits into from
Sep 2, 2015

Conversation

armandabric
Copy link
Member

  • Add check cyclic dependencies on call hook (not needed)
  • Rename the ServiceArgumentCollection to ArgumentCollection and ServiceArgument to Argument

@armandabric armandabric changed the title Implement post instantiation method call on service [WIP] Implement post instanciation method call on service Aug 31, 2015
*/
Call.prototype.trigger = function trigger(container, instance) {
if (!instance[this.methodName]) {
throw new Error('Can\'t call the given method: "' + this.methodName + '" does not exist on the given instance.');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So... we will never know until runtime that the method is unknown isn't it? So... the big question is... do we have to be able to launch some kind of checks at container's instanciation? In this case we maybe should collect all checks in a heap and delegate the exception throwing to a dedicated class that will be specialized in heap checks assertions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've add check on container initialization. These check stay "just in case".

@armandabric armandabric changed the title [WIP] Implement post instanciation method call on service Implement post instanciation method call on service Sep 1, 2015
@@ -15,6 +15,11 @@ var Container = function Container(serviceDefinitionCollection, parameterCollect
this.parameterCollection = parameterCollection;

this.serviceStorage = new ServiceStorage();

if (process.env.NODE_ENV === 'development') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, instead of being hard coupled to NODE_ENV here... this could become an argument of the Container constructor?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like:

function Container(serviceDefinitionCollection, parameterCollection, boolean: validateDefinitions)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ho 😱 That's a really good idea!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will need to add it to the ContainerFactory too.

@shouze
Copy link
Member

shouze commented Sep 2, 2015

LGTM, good to squash then merge! 👍

armandabric added a commit that referenced this pull request Sep 2, 2015
Implement post instanciation method call on service
@armandabric armandabric merged commit 89237bb into ubirak:master Sep 2, 2015
@armandabric armandabric deleted the hooks branch September 2, 2015 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants