Skip to content

Conversation

@EdaCZ
Copy link
Contributor

@EdaCZ EdaCZ commented Mar 1, 2016

Purpose: throw an Exception when you have typo in the name of parameter in factory method (or constructor of factory product).

Related: #60

throw new ServiceCreationException("Type hint for \${$param->getName()} in $interface::$methodName() doesn't match type hint in $class constructor.");
}
$def->getFactory()->arguments[$arg->getPosition()] = self::literal('$' . $arg->getName());
} elseif (!$def->getSetup()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused, how is setup of service relevant to it's constructor parameters and to parameters of create method of the factory interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When implementing generated factory there are only two ways to use parameters in create method:
a) in constructor of created object.
b) in setup directives.
Or am I wrong?

Copy link
Contributor

Choose a reason for hiding this comment

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

@fprochazka When the parameter from create method did not exist in the constructor it failed with no clear exception. I was trying to solve this when #60 was reported but my solution broke some tests - the parameter can be used in setup instead of the constructor. With this condition it should work fine to throw the exception.

Copy link
Member

Choose a reason for hiding this comment

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

Exception message should be even better :) IMHO typo is in interface, not in constructor, because constructor is primary and interface secondary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will try to implement "Did you mean" feature.
This would be ok?
Unused parameter $baz when implementing method Bad4::create(). Typo in Bad4::create() parameters? Did you mean $bar?

Copy link
Member

Choose a reason for hiding this comment

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

What about: Unused parameter $baz when implementing method Bad4::create(). Did you mean $bar in Bad4?


Maybe more comples:

Unused parameter $baz when implementing method Bad4::create(). Did you mean Bad4::create($bar)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure, what to choose.
So, @dg, please choose one and I'll update commit.

@enumag
Copy link
Contributor

enumag commented Mar 2, 2016

Good job @EdaCZ. 👍

@xificurk
Copy link
Contributor

xificurk commented Mar 2, 2016

Improvement suggestion: Add "Did you mean ...?" feature to the exception message.

@EdaCZ
Copy link
Contributor Author

EdaCZ commented Mar 10, 2016

@dg I've updated commit with "Did you mean feature". Is it ok now?

@dg
Copy link
Member

dg commented Mar 10, 2016

Great! Thanks

dg added a commit that referenced this pull request Mar 10, 2016
Check for unused parameter when generating factory
@dg dg merged commit 3529820 into nette:master Mar 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants