Skip to content
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

[Simplify] Clean-up report builder to make it easier to read and prep… #35

Merged
merged 1 commit into from
Apr 23, 2023

Conversation

Nava2
Copy link
Collaborator

@Nava2 Nava2 commented Apr 23, 2023

…are for konvert

Extracted from #29

Comment on lines +24 to +30
File directoryPathWhereAssertionFilesAreGenerated
Collection<String> inputPackages
Collection<String> inputClasses
Exception exception
Collection<TypeToken<?>> excludedClassesFromAssertionGeneration
final List<String> userTemplates
final Set<String> inputClassesNotFound
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All of these replace needless get() methods

private Collection<TypeToken<?>> excludedClassesFromAssertionGeneration
private Set<String> inputClassesNotFound
private List<String> userTemplates
private static final String Line = System.lineSeparator()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this makes the append(Line) easier to read than append(System.lineSeparator())

reportBuilder.append(INDENT).append("Given classes : ").append(Arrays.toString(inputClasses))
reportBuilder.append(System.lineSeparator())
}
if (isNotEmpty(inputPackages as String[])) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These incantations were replaced with !field?.isEmpty() which groovy coaleces null -> false and is much easier to grep.

@@ -17,24 +17,25 @@ import org.assertj.assertions.generator.AssertionsEntryPointType

import static com.google.common.collect.Maps.newTreeMap
import static com.google.common.collect.Sets.newTreeSet
import static org.apache.commons.lang3.ArrayUtils.isNotEmpty
import static org.apache.commons.lang3.StringUtils.remove
import static org.apache.commons.lang3.exception.ExceptionUtils.getStackTrace

class AssertionsGeneratorReport {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Review this with whitespace checks off, it's much more obvious what I changed.

reportBuilder.append("No custom assertions files generated for the following input classes as they were not found:\n")
for (String inputClassNotFound : inputClassesNotFound) {
reportBuilder.append(INDENT).append(inputClassNotFound).append(System.lineSeparator())
new StringBuilder().tap {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The tap method is identical to Kotlin's apply. Ideally, I would rewrite the methods that add more content as Extension functions, but we can't because Groovy has a limitation where extensions can only be static and defined on separate objects. More or less a needless limitation from a language standpoint, but I'm sure there's a reason technically. Kotlin doesn't have this limitation 👍🏻

@Nava2 Nava2 requested a review from scordio April 23, 2023 13:35
@Nava2 Nava2 merged commit 50f8567 into main Apr 23, 2023
@Nava2 Nava2 deleted the kevin/clean-up-report-prep branch April 23, 2023 17:51
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.

1 participant