Skip to content

Conversation

zluiten
Copy link
Contributor

@zluiten zluiten commented Apr 4, 2022

Closes #24

The OrderByClause is basically a duplicate of the FilterClause value object. Which pops up the question: should both VO's be replaced by a new more generic "SqlClause"...

@zluiten zluiten force-pushed the feature/extract-orderby-processor branch 2 times, most recently from a6e7ef2 to ca8bfa6 Compare April 4, 2022 07:51
@codeliner
Copy link

codeliner commented Apr 4, 2022

We're fans of explicit code. Even thought both VOs look the same they have different meaning (especially as a type in the respective processor interface). Therefore, I think we should not introduce a SqlClause

@zluiten
Copy link
Contributor Author

zluiten commented Apr 4, 2022

I can follow that reasoning. I prefer explicity typing myself as well.

@zluiten zluiten marked this pull request as ready for review April 4, 2022 15:24
@zluiten zluiten marked this pull request as draft April 4, 2022 15:25
@zluiten zluiten force-pushed the feature/extract-orderby-processor branch from ca8bfa6 to 909c5d4 Compare April 4, 2022 16:16
@zluiten zluiten marked this pull request as ready for review April 4, 2022 16:16
@zluiten zluiten requested a review from codeliner April 4, 2022 16:16
@zluiten
Copy link
Contributor Author

zluiten commented Apr 4, 2022

I've added 2 unit tests

@codeliner codeliner merged commit 60552f5 into event-engine:master Apr 4, 2022
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.

Extract order by processing logic
2 participants