Skip to content

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