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

Add 'duplicatesAndUnique' method to analyze haiku text characters #295 #323

Merged
merged 2 commits into from
Sep 22, 2023

Conversation

NiveditaMishraCoding
Copy link
Contributor

Add 'duplicatesAndUnique' method to analyze haiku text characters

@NiveditaMishraCoding NiveditaMishraCoding changed the title Add 'duplicatesAndUnique' method to analyze haiku text characters Add 'duplicatesAndUnique' method to analyze haiku text characters Issue #295 Sep 22, 2023
@NiveditaMishraCoding NiveditaMishraCoding changed the title Add 'duplicatesAndUnique' method to analyze haiku text characters Issue #295 Add 'duplicatesAndUnique' method to analyze haiku text characters #295 Sep 22, 2023
Copy link

@Sweta716 Sweta716 left a comment

Choose a reason for hiding this comment

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

Consider using more descriptive variable names. While chars is clear, charCounts, duplicates, and unique are more explicit and make the code easier to understand.

@NiveditaMishraCoding
Copy link
Contributor Author

Consider using more descriptive variable names. While chars is clear, charCounts, duplicates, and unique are more explicit and make the code easier to understand.

The variable names were already given. I don’t wanted to make changes on existing variables.

Copy link

@rachanavishwanath rachanavishwanath left a comment

Choose a reason for hiding this comment

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

Please rebase and squash the commits

import org.eclipse.collections.impl.factory.SortedBags;

import java.util.List;

Choose a reason for hiding this comment

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

Why are we removing this import? Is it unused?

.map(Character::toLowerCase) // Convert to lowercase
.collect(Collectors.groupingBy(c -> c, Collectors.counting()));

Map<Character, Long> duplicates = chars.entrySet().stream()

Choose a reason for hiding this comment

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

Can you please rename this to duplicateChars

.filter(entry -> 1L < entry.getValue()) // Filter duplicates
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));

Set<Character> unique = chars.entrySet().stream()

Choose a reason for hiding this comment

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

Same as above, please rename to uniqueChars to be specific and readable code

@@ -41,8 +41,8 @@ public void distinctLetters()
Assertions.assertEquals("breakingthoupvmwcdflsy", distinctLetters);
}

// @Test // Uncomment once duplicatesAndUnique is implemented for JDK
// @Tag("SOLUTION")
@Test // Uncomment once duplicatesAndUnique is implemented for JDK

Choose a reason for hiding this comment

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

Please remove the comment as we would not need it post the PR merge

@prathasirisha
Copy link
Contributor

Thank you @NiveditaMishraCoding for your contributions!!

@prathasirisha prathasirisha merged commit 3c88b1d into eclipse:master Sep 22, 2023
5 checks passed
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.

5 participants