For an assignment in my compsci class, I need to use hashmaps to convert a message into an encrypted message. We're told to do this by replacing all characters in a given message string with characters in a substitution alphabet that consists of lowercase letters from a-z. Also, the substitution should be done in alphabetical order.

So for example if we're given the message "hello", our cipher should map h to a, e to b, l to c, and o to d, giving us the string abccd. In theory it doesn't seem that hard, but I'm having a lot of trouble with the actual conversion of characters. I've put a comment where I was intending to do the actual conversions, but so far all I'm getting returned is a bunch of a's and b's put together. What am I doing wrong here?

Thanks in advance for any help.

EDIT: Removed some redundant code thanks to @taha

public class Encryption {
    public String encrypt(String message){
        StringBuilder convertedMessage = new StringBuilder();
        int counter = 0;
        char[] alphabet = "abcdefghijklmnopqrstuvwxyz".toCharArray();

        HashMap<Character, Character> converter = new HashMap<>();
        String messageholder = message.toLowerCase();
        char[] charMessage = messageholder.toCharArray();

//this statement is what's giving me an aneurysm
        for (char i : charMessage ){
            converter.putIfAbsent(i, alphabet[counter]);
            counter  ;
        }

        for (Character c : converter.keySet()){
            char value = converter.get(c);
            convertedMessage.append(value);
        }

        return convertedMessage.toString();
    }
}

CodePudding user response：

The code seems to have 2 major issues:

1. There is an attempt to remove non-alphabet characters from the input message
2. The second loop is not needed at all, and the encrypted message should be built while iterating the input message as soon as the cipher character is selected.

public static String encrypt(String message){
    StringBuilder convertedMessage = new StringBuilder();
    int counter = 0;
    char[] alphabet = "abcdefghijklmnopqrstuvwxyz".toCharArray();

    HashMap<Character, Character> converter = new HashMap<>();
    // keep only lower-case English letters
    message = message.replaceAll("[^A-Za-z]", "").toLowerCase();

    for (char i : message.toCharArray()) {
        converter.putIfAbsent(i, alphabet[counter  ]);
        convertedMessage.append(converter.get(i));
    }

    return convertedMessage.toString();
}

// Test
System.out.println(encrypt("Hello,World!"));

Output:

abccefehcj

However, the code above updates the counter each time a character from the input message is looked up and therefore some values may be skipped. For example, d is skipped because counter is incremented for the second l in hello. Similarly, g is skipped for the second occurrence of o in world, etc.

So, it could make sense to use method Map::computeIfAbsent instead of Map::putIfAbsent which calls a function to modify the counter only when needed.

In this case, counter should be implemented as an effectively final variable because it is used in a lambda function - either as an array or AtomicInteger:

int[] counter = new int[1]; // or AtomicInteger counter = new AtomicInteger();

for (char i : charMessage ) {
    converter.computeIfAbsent(i, k -> alphabet[counter[0]  ]);
// for AtomicInteger converter.computeIfAbsent(i, k -> alphabet[counter.getAndIncrement()]);
    convertedMessage.append(converter.get(i));
}

Then the encrypted string for Hello World! looks like this:

abccdedfcg