ahem. edited out where I wrongly pointed out that new HashMap<>() was incorrect, eventhough it actually is ;)
As was pointed out, temp is a name you may want to avoid unless you are swapping members and you are using a temporary storage.
your occur only is used inside this method so it can be situated inside that scope.
if(!tempMap.containsKey(tchars[i])){
int occur = 0;
this means you do not have to set it to 0 again. Inside a specific scope, it's not an expense to have another stack variable. More so if it gets released quickly because you exit the scope.
this line is unnecessary:
String temp = target;
You never overwrite target or temp afterwards, so you simply can use target.
One thing that you could use a local variable for though it tchars[i]. you use that a lot and it refers to the same character the whole time.
char currentCharacter = tchars[i];
will mean you can talk about 'currentCharacter' instead which at least is descriptive.
even easier, since all you do is loop over the characters in tchars, you can do:
for (char currentCharacter : tchars) {
}
You don't use i for anything else, so now you don't even need it anymore.
not everyone likes assignment inside a while, but this can be made simpler:
int index = temp.indexOf(tchars[i]);
while (index != -1) {
occur++;
index = temp.indexOf(tchars[i], index + 1);
}
it can be:
int index = -1;
while ( (index = target.indexOf(tchars[i], index + 1)) != -1) {
occur++;
}
Now you don't have 2 types of indexOf floating around which will have to be kept in synch.
I know, you aren't gaining much from it but it's generally better not to have 2 versions of the same operation. Once might get refactored and the other might be left in there.
anyways I admit, this is not the strongest suggestion I am making.
In any case, I think your occurance is miscalculating still, if source contains a character twice, then you will still add the number of that character 2 times. This could be intentional ofcourse.
Assuming you only count each unique character from source in target, I would use 2 loops to generate this.
String target = "alex alexander";
String source = "ardx";
Map<Character, Integer> characterCount = new HashMap<Character, Integer>();
for (char sourceCharacter : source.toCharArray()) {
characterCount.put(sourceCharacter, 0);
}
and then do your counting by going over each char in target. if it is a character we are counting then the char exists as key in the map. If it's not a character we count, we skip to the next character.
int totalCount = 0;
for (char targetCharacter : target.toCharArray()) {
if (!characterCount.containsKey(targetCharacter)) {
continue; // not a character we are counting.
}
// can be done in 1 statement ofcourse
int currentCount = characterCount.get(targetCharacter);
characterCount.put(targetCharacter, currentCount + 1);
totalCount++;
}
System.out.println(totalCount);
This way you don't need to run through the target output twice (once to see if it is a character to count, and then to do the actual counting)
I'm not a great fan of doing totalCount++ in the main loop. I'd rather use a 3rd loop that would total up all the counts. This way if the second loop (the counting loop) ever changes, you don't mess up the totalling up.
so something like:
int totalCount = 0;
for (int currentCount : characterCount.values()) {
totalCount += currentCount;
}
System.out.println(totalCount);
it's another looping but the loop is fast enough and if you ever want to move the 'counting' loop into it's own method, you don't need worry about the total.
