Mdthbs

I wrote a small program for hyperskill/jetbrains academy to encrypt the message "we found a treasure!" and print only the ciphertext (in lower case) by replacing each letter with the letter that is in the corresponding position from the end of the English alphabet (a→z, b→y, c→x, ... x→c, y →b, z→a) and without replace spaces or the exclamation mark.

I would like code review comments in terms of correct use of java.

package encryptdecrypt;



public class Main {



    final static int lower_case_a = 'a';

    final static int lower_case_z = 'z';



    public static void main(String[] args) {

        String s = "we found a treasure!";



        StringBuffer reverse = new StringBuffer();



        for (int i = 0; i < s.length(); i++) {

            char c = s.charAt(i);



            if ((int)c >= lower_case_a && (int)c <= lower_case_z)

                reverse.append((char)(lower_case_z - c + lower_case_a));

            else

                reverse.append((char) c);

        }

        System.out.println(reverse.toString());

    }

 }

Some my suggestions about your code: instead of iterating over the string s because you are not using the i index inside your loop you could iterate over the char array from s like below:

char[] arr = s.toCharArray();

for (char c : arr) { //here the body }

Inside the loop you can use Character.isLowerCase method and you can use a ternary operator to append your character to the StringBuffer evaluating if it is a lowercase character or not:

String s = "we found a treasure!";

char[] arr = s.toCharArray();

StringBuffer reverse = new StringBuffer();



for (char c : arr) {

    char ch = Character.isLowerCase(c) ? (char)(219 - c) : c; //<-- 219 = int('a') + int('z')

    reverse.append(ch);

}

Inside the loop I put 219 because it is a const value so it is not useful recalculate it for every iteration of the loop.

With

final static int lower_case_a = 'a';

final static int lower_case_z = 'z';

While it's arguably good that you have 'a' and 'z' stored as variables and not floating around, I don't think they're named well. lower_case_a tells you exactly as much as 'a' itself, so you aren't really gaining much. The purpose of the variable would make for a better name. Also, since those are constants, they should be upper case, and underscore-separated. I'd change those lines to:

final static int ALPHABET_UPPER_BOUND = 'a';

final static int ALPHABET_LOWER_BOUND = 'z';

Also, as mentioned in the above link, Java uses camelCase, not snake_case.

if ((int)c >= lower_case_a && (int)c <= lower_case_z)

    reverse.append((char)(lower_case_z - c + lower_case_a));

else

    reverse.append((char) c);

This has a few notable things:

• Do not omit braces here. I won't go as far as to say that they should never be omitted, but the cases where it's appropriate to not use them are pretty slim. If you ever make changes to those lines and don't notice that you didn't use braces, at bst you'll need to deal with syntax errors, and at worst you'll get weird behavior that you'll need to debug. Just use them. It costs next to nothing and prevents a range of errors.




• Instead of writing (int)c in a few places, I'd probably instead save that in a variable.




• (char) c isn't necessary. c is already a char.




• You could use a ternary since each branch is just giving its data to append.

Something closer to:

int code = c;

char newChar = (code >= ALPHABET_UPPER_BOUND && code <= ALPHABET_LOWER_BOUND)

                 ? (lower_case_z - c + lower_case_a) : c;



reverse.append(newChar);

Although whether you'd want to use a ternary here is subjective. That line is a little long.

I also agree with @dariosicily though. You never use i for anything other than indexing, so you might as well just use a foreach loop and iterate the string directly.