Remove return type dependency of filter step #4
Conversation
| @@ -39,7 +40,7 @@ public function process($item, callable $next) | |||
| { | |||
| foreach (clone $this->filters as $filter) { | |||
| if (false === call_user_func($filter, $item)) { | |||
| return false; | |||
| throw new FilterException(); | |||
There was a problem hiding this comment.
This is not a good idea, this will decrease the performance a lot.
There was a problem hiding this comment.
I would be happy with any alternative which does not mess with return values.
There was a problem hiding this comment.
The thing is, i agree if we goes with good design this should throw an exception but if we decide to focus on performance this shouldn't be modified.
Is there any alternative to throw an exception? I really like good software design, but performance is also importance.
There was a problem hiding this comment.
I am not sure, I can't think of any clean solution, only ugly ones (with variables passed by reference at some point)
I don’t agree that if the filter is used on many (or all) items, the filtering should be done outside of Port. We have the filters exactly in order to enable users to filter and write their data in one workflow.
One of the focus points of ddeboer/data-import was not performance per se, but rather using as little memory as possible. After all, when processing large amounts of data with PHP, memory can be a problem. |
|
Actually, I kind thought myself with this. @Baachi is right, exceptions would be a huge performance hit, which we probably don't want, even if performance is not our number one priority. Also, this exception is probably still too much filter specific, so I propose the following: Define return status codes in the |
|
@sagikazarmark we can also use constants for that which have these values. |
|
In case it wasn't clear: I meant them to be constants with the given integer values. I would rather work with integer than string status codes. |
|
Sounds good. 👍 |
We have already been talking about using exceptions in a workflow and we agreed we should keep their number low as exceptions require more resources. I am still opening this PR now, because I think we should remove the return type dependency from steps.
Thinking about filters: if a filter must be used too much times, then the workflow is possibly badly constructed. Big data feeds should be filtered as much as possible prior to processing with Port.
What do you think?