Skip to content

Refactor SummaryMetadataEnricher for improved readability #394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

zbqmgldjfh
Copy link
Contributor

We've used methods throughout the code to give it meaning.
The old code was very hard to read because you had to read so many implementations.

You can definitely see the improvement in readability after refactoring.

The test was performed using the MetadataTransformerIT class.

@zbqmgldjfh zbqmgldjfh changed the title Refactor SummaryMetadataEnricher for Improved Readability Refactor SummaryMetadataEnricher for improved readability Mar 4, 2024
@tzolov tzolov added this to the 1.0.0-M1 milestone Mar 6, 2024
@tzolov tzolov modified the milestones: 1.0.0-M1, 1.0.0-M2 May 28, 2024
@markpollack
Copy link
Member

markpollack commented Jul 24, 2024

I think this refactoring goes a bit too far for my taste, it decomposes it too much and passing around the index makes it more confusing. I've made a small change

		for (int i = 0; i < documentSummaries.size(); i++) {
			Map<String, Object> summaryMetadata = getSummaryMetadata(i, documentSummaries);
			documents.get(i).getMetadata().putAll(summaryMetadata);
		}

in this commit 7775a76 as i think this helps with clarity.

Thanks for the PR and sorry it took so long to get around to it.

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.

3 participants