patternjavaMinor
Find sum of number of times that each character of source occurs in the target
Viewed 0 times
findnumbertheeachtargetsourcecharacterthatsumtimes
Problem
I wrote the following code in response to this question "find sum of number of times that each character of source occurs in the target", is there any better solution for it?
Analysis
Code
Output
Analysis
Target is alex alexander
Source is ardx
Characters of source that exist in target are like a--x-a--xa-d-r
Total is 7.
Code
String target = "alex alexander";
String source = "ardx";
char[] a = source.toCharArray();
int occurrences = 0;
for (int i = 0; i < a.length; i++) {
String t = target;
int index = t.indexOf(a[i]);
while (index != -1) {
occurrences++;
t = t.substring(index + 1);
index = t.indexOf(a[i]);
}
}
System.out.println(occurrences);
}Output
7
Solution
Your code is neat, well structured, and generally very readable. People solving this problem are often beginners, and your code, for a beginner, is good.
My only style complaint is the use of
Your algorithm is technically a nested loop. You loop through each character in the source, and for each of the source characters, you loop through the characters in target.
Note that the indexOf function is essentially a loop, it starts at the start index, and loops until it gets to the end, or the character. Your while loop is just a way to 'pause' the loop at significant places.
So, your algorithm will scan the target one time for each source character. Each time there is a match, you increment the
Your algorithm could be made more apparent if you were to make the inner loop more obvious:
That makes the logic more visible, and removes the call to indexOf....
Obviously, it would help to convert the target to a char array outside the loop:
and then the inner loop would be:
There is a potential bug with your solution.... if a character is duplicated in the source, you will double-count it in the result.
You will want to deal with that.
There is also a possibly more efficient algorithm which would be useful for large strings, but, with anything less than say 100 chars, I would not bother. That algorithm would require setting up a HashMap or other data structure from the source chars, and then just looping once through the target chars to get the counts.
My only style complaint is the use of
a for the array variable name.Your algorithm is technically a nested loop. You loop through each character in the source, and for each of the source characters, you loop through the characters in target.
Note that the indexOf function is essentially a loop, it starts at the start index, and loops until it gets to the end, or the character. Your while loop is just a way to 'pause' the loop at significant places.
So, your algorithm will scan the target one time for each source character. Each time there is a match, you increment the
occurrences.Your algorithm could be made more apparent if you were to make the inner loop more obvious:
for (int j = 0; j < target.length(), j++) {
if (target.charAt(j) == a[i]) {
occurrences++;
}
}That makes the logic more visible, and removes the call to indexOf....
Obviously, it would help to convert the target to a char array outside the loop:
char[] tchars = target.toCharArray();and then the inner loop would be:
for (int j = 0; j < tchars.length, j++) {
if (tchars[j] == a[i]) {
occurrences++;
}
}There is a potential bug with your solution.... if a character is duplicated in the source, you will double-count it in the result.
You will want to deal with that.
There is also a possibly more efficient algorithm which would be useful for large strings, but, with anything less than say 100 chars, I would not bother. That algorithm would require setting up a HashMap or other data structure from the source chars, and then just looping once through the target chars to get the counts.
Code Snippets
for (int j = 0; j < target.length(), j++) {
if (target.charAt(j) == a[i]) {
occurrences++;
}
}char[] tchars = target.toCharArray();for (int j = 0; j < tchars.length, j++) {
if (tchars[j] == a[i]) {
occurrences++;
}
}Context
StackExchange Code Review Q#63085, answer score: 5
Revisions (0)
No revisions yet.