Skip to content

Streamline formatter implementation#4044

Merged
koppor merged 5 commits into
masterfrom
feature/abstract-formatter
May 24, 2018
Merged

Streamline formatter implementation#4044
koppor merged 5 commits into
masterfrom
feature/abstract-formatter

Conversation

@koppor

@koppor koppor commented May 21, 2018

Copy link
Copy Markdown
Member

This builds on #4043 and streamlines the code of the "formatter" framework a bit.

  • Introduce AbstractFormatter to ensure that hashCode and equals are implemented the same way
  • Introduce a test to ensure that getKey() returns a unique key

@koppor koppor added the dev: code-quality Issues related to code or architecture decisions label May 21, 2018
@koppor koppor mentioned this pull request May 21, 2018
6 tasks
@Siedlerchr

Copy link
Copy Markdown
Member

Why not adding the methods implemented to the interface?

@koppor

koppor commented May 22, 2018

Copy link
Copy Markdown
Member Author

grafik

@tobiasdiez

Copy link
Copy Markdown
Member

Why not make Formatter an abstract class?

@koppor

koppor commented May 22, 2018

Copy link
Copy Markdown
Member Author

If this get merged, please with normal merge. Then the target branch of #4045 has to be changed to master and then can be merged correctly. 😇

@koppor

koppor commented May 22, 2018

Copy link
Copy Markdown
Member Author

@stefan-kolb introduced the interface paradigma for formatters in 2015. The interface resides in our model package, whereas the concrete implementations reside in the logic package. In c09e472, I changed the interface to an abstract class.

assertFalse(getFormatters().collect(Collectors.groupingBy(
formatter -> formatter.getKey(),
Collectors.counting()
)).entrySet().stream().anyMatch(e -> e.getValue() > 1));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In case somebody adds a new formatter with the same key, this test fails but the error message does not say why. I would change anyMatch to filter and compare (assertEquals) the resulting map to an empty map.

@koppor koppor changed the base branch from feature/title-enclosing to master May 24, 2018 14:45
@koppor koppor merged commit 5f21d47 into master May 24, 2018
@stefan-kolb stefan-kolb deleted the feature/abstract-formatter branch May 25, 2018 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev: code-quality Issues related to code or architecture decisions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants