Making Anagram Problem Implementation

Question

Problem Statement

Alice is taking a cryptography class and finding anagrams to be very useful. We consider two strings to be anagrams of each other if the first string's letters can be rearranged to form the second string. In other words, both strings must contain the same exact letters in the same exact frequency. For example, bacdc and dcbac are anagrams, but bacdc and dcbad are not.

Alice decides on an encryption scheme involving two large strings where encryption is dependent on the minimum number of character deletions required to make the two strings anagrams. Can you help her find this number?

Given two strings, a and b, that may or may not be of the same length, determine the minimum number of character deletions required to make a and b anagrams. Any characters can be deleted from either of the strings.

Solution

//Valid Test Case For Reference
static void ransomeNoteTrue() {
    String a = "cde";
    String b = "abc";
    int result = makeAnagram(a, b);
    int expected = 4;
    assert (result == expected);
}

static int makeAnagram(String a, String b) {
    Character[] aArray = a.chars().mapToObj(x -> (char) x).toArray(Character[]::new);
    Character[] bArray = b.chars().mapToObj(x -> (char) x).toArray(Character[]::new);
    Map<Character, Long> aMap = getFrequencyMapFromArray(aArray);
    Map<Character, Long> bMap = getFrequencyMapFromArray(bArray);

    return countError(aMap, bMap);
}

private static int countError( Map<Character, Long> aMap, Map<Character, Long> bMap ) {
    long deletedChars = 0;

    for( char ch='a'; ch<='z'; ch++) {

        if ( aMap.containsKey(ch) && !bMap.containsKey(ch) )
            deletedChars += aMap.get(ch);

        else if( bMap.containsKey(ch) && !aMap.containsKey(ch) )
            deletedChars += bMap.get(ch);

        else if( bMap.containsKey(ch) && aMap.containsKey(ch) )
            deletedChars += Math.abs( aMap.get(ch) - bMap.get(ch));
    }

    return (int)deletedChars;
}

private static Map<Character, Long> getFrequencyMapFromArray(Character[] arr) {
    return Arrays.stream(arr)
            .collect(Collectors.groupingBy(Function.identity(), Collectors.counting()));
}

The main function to review is makeAnagram().

Looking forward to your reviews on -

1. What other improvements can be done?
2. What other ways could be there to the problem, as the current approach is brute force?

Thank you.

Answer 1

Do not map to array

I appreciate that streaming strings is a little messy. However, there is no need to create an array; instead we can just transform the stream. I would remove the arrays and change getFrequencyMap to take a String.

Use Map::getOrDefault to simplify countError

In particular, if you do aMap.getOrDefault(ch, 0) then the Math.abs case always works.

Unlike @DapperDan's solution, this still requires looping through all 26 characters; however, the logic is a lot simpler. At least on my system, my approach is faster, even in you example when each map has only 3 elements. The cost of iterating through 26 characters is very low compared to map operations.

Use of longs

I notice you use Longs throughout (presumably due to Collectors.counting() returning a Long) but then cast to int at the end. Why not just return a long?

Stream rangeClosed vs for loop

Mostly a matter of style, but it is easy to replace the for loop with a stream.

Code

Applying these changes should yield something like

static long makeAnagram(String a, String b) {
    return countError(getFrequencyMap(a), getFrequencyMap(b));
}

private static long countError(Map<Character, Long> aMap, Map<Character, Long> bMap) {
    return IntStream.rangeClosed('a', 'z')
        .mapToObj(x -> Character.valueOf((char)x))
        .mapToLong(c -> Math.abs(aMap.getOrDefault(c, 0l) - bMap.getOrDefault(c, 0l)))
        .sum();
}

private static Map<Character, Long> getFrequencyMap(String s) {
    return s.chars()
        .mapToObj(x -> Character.valueOf((char)x))
        .collect(Collectors.groupingBy(Function.identity(), Collectors.counting()));
}

Answer 2

This is actually a pretty solid solution (I am particularly fond of your getFrequencyMapFromArray()). The only "brute force" section I notice is your for loop in countError() loops through all letters a-z, rather than only the letters observed.

You could reduce this to only those present in the maps by using the map.keySet() and looping through both maps' key sets. As a bonus, when looping through the first keySet, map.remove() the keys present in both from the next map, to reduce your duplication even further! Should look something like this:

private static int countError( Map<Character, Long> aMap, Map<Character, Long> bMap ) {
    long deletedChars = 0;
    for(Character key : aMap.keySet()){
        if (!bMap.containsKey(key) ) {
            deletedChars += aMap.get(key);
        } else if( bMap.containsKey(key)){
            deletedChars += Math.abs( aMap.get(key) - bMap.get(key));
            bMap.remove(key);//Remove here bc we have already checked this.
        }
    }
    for(Character key : bMap.keySet()){
        if(!aMap.containsKey(key)){
            deletedChars += bMap.get(key);
        }//No need to check for 'both maps contain' bc we have removed all such cases

    }
    return (int)deletedChars;
}