[WIP] Initial release #1
Conversation
|
ping @danizord This is just a backport of my refactor but with the main classes for now. I'd like to clearly understand your point of view regarding the factory. Right now, as you can see the problem is that I need to inject ValdiatorChain and FilterChain in all inputs. Actually, they are optional (like in ZF2), but the problem is that if you let the Input creates itself those two things, you will loose any custom valdiators/filters, which lead to a lot of problems, especially for beginners that do not understand why. What do you think? I'd like to quickly have a working version for those two classes. |
| protected $factory; | ||
|
|
||
| /** | ||
| * @var InputCollectionInterface[]|InputInterface[] |
There was a problem hiding this comment.
Since InputCollectionInterface extends InputInterface it can be @var InputInterface[].
There was a problem hiding this comment.
Actually I prefer to use that, so we can have proper hints from IDE when looping.
There was a problem hiding this comment.
That's incorrect. You should not call any method from InputCollectionInterface inside a loop if it don't contains only instances of InputCollectionInterface (:
|
Didn't like that runAgainst method. I would prefer ->filter() or something else. |
|
@fabiocarneiro , no the runAgainst is definitely the big +1 of this refactor, because it allows completely stateless input / input collection. |
| class InputCollection extends Input implements InputCollectionInterface | ||
| { | ||
| /** | ||
| * @var InputCollectionInterface[]|InputInterface[] |
|
@bakura10 i was not talking about removing the method. I was talking about its name. runAgainst seems weird to me. |
| */ | ||
| public function getFilterChain() | ||
| { | ||
| return $this->filterChain ?: new FilterChain(); |
There was a problem hiding this comment.
if (null === $this->filterChain) {
$this->filterChain = new FilterChain();
}There was a problem hiding this comment.
Ha yeah, I get your point, sorry.
|
lol we had a debate about this and runAgainst was the best name we found :/. If you have a better idea, go on! |
|
|
||
| if ( | ||
| $this->validatorChain->isValid($filteredValue, $context) | ||
| || (null === $filteredValue && $this->allowEmpty) |
There was a problem hiding this comment.
Should probably check it first than validator chain.
| { | ||
| $type = isset($specification['type']) ? $specification['type'] : Input::class; | ||
|
|
||
| if ($type instanceof InputInterface) { |
|
I really have no idea about how to deal with the "required" parameter (adding the NotEmpty validator). In ZF2 the code seems strange: https://github.com/zendframework/zf2/blob/master/library/Zend/InputFilter/Input.php#L359 If for whatever reason you set required back to false, the validators is never removed. |
| * Uses a concept of "InputFilterResult" | ||
| * Renames to make it easier to understand (previous "InputFilter" has been renamed to "InputCollection") | ||
| * Behaviour is now much more predictable | ||
| * ... and a lot of things! |
No description provided.