patternjavaMinor
Find common "characters" in 2 given strings (rev1)
Viewed 0 times
rev1commonfindcharactersstringsgiven
Problem
I would like a review of:
In particular:
Less interesting to me:
A glance at program output from online execution
I have rewritten the code below, including feedback, as Rev2 @ Find common "characters" in 2 given strings (rev2), follow the link to look at the updated version
``
//
// String string1 = "\uD835\uDC00", string2 = "\uD835\uDC01";
//
// string1 and string2 share no characters in the intended acceptation of
// "character".
String shortestArgument, longestArgument;
if (string1.length() codePointsSeen =
shortestArgument.codePoints()
.bo
- Correctness, especially around edge/corner cases
- Efficiency
- Readability and/or style
- Appropriate use of Java idioms and data structures
In particular:
- Have I missed typical idioms/patterns useful to solving the problem?
- Could I have reused existing/popular framework/library code?
Less interesting to me:
- Feedback on the test harness logic in
main()(I could have used JUnit)
A glance at program output from online execution
I have rewritten the code below, including feedback, as Rev2 @ Find common "characters" in 2 given strings (rev2), follow the link to look at the updated version
``
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
class CommonCharacters {
@SuppressWarnings("boxing")
private static String commonCharactersOf(String string1, String string2) {
// Requirement
//
// Always return lowercase versions of common characters. e.g.:
//
// OK: (a, a) -> a
// OK: (a, A) -> a
// OK: (A, A) -> a
// No: (A, A) -> A
// No: (aA, aA) -> aA
//
// Requirement
//
// Return common characters joined in a String, preserving the order in
// which they appeared in the longest argument, or in the first argument if
// the arguments are of the same length.
//
// Requirement
//
// Handle "characters" (i.e. code points) outside the Basic Multilingual
// Plane (BMP), including characters from Supplementary Planes.
// There should be no char' or `Character' based "false positives". i.e.://
// String string1 = "\uD835\uDC00", string2 = "\uD835\uDC01";
//
// string1 and string2 share no characters in the intended acceptation of
// "character".
String shortestArgument, longestArgument;
if (string1.length() codePointsSeen =
shortestArgument.codePoints()
.bo
Solution
I like that you use Streams. But I think you should use even more of their functionality:
-
You could have used
-
You use
-
For the final output, you can use
You spend many lines defining the shortest/longest strings, but I don't see how that is relevant. You could just assign any string to any of those and the algorithm would still work.
I might have written the program a bit differently. I would define a function that takes a string and returns the lower case version with no duplicate, in the same order, as an
-
You could have used
map to get the lower case for longestArgument. Or maybe it would have been simpler to just use longestArgument.toLowerCase(), and same for shortestArgument.-
You use
distinct() to remove duplicate, but you could have gotten rid of the main for-loop entirely by also using filter(...) where the filter uses a predicate built from the values in shortestArgument.-
For the final output, you can use
codePointStream.toArray() to create an int[] and then use String(codePointArray, 0, codePointArray.length). You could instead use intStream.collect(...) to perform a reduction which would fill and return a StringBuilder, but it would end up quite ugly.You spend many lines defining the shortest/longest strings, but I don't see how that is relevant. You could just assign any string to any of those and the algorithm would still work.
I might have written the program a bit differently. I would define a function that takes a string and returns the lower case version with no duplicate, in the same order, as an
IntStream. I would apply that function at the start on both input strings. And then apply filter on one of them, with the predicate built from the values of the other.Context
StackExchange Code Review Q#106473, answer score: 4
Revisions (0)
No revisions yet.