Cäsar-Verschlüsselung

[Kar]

Guten Abend,
ich hatte die Hausaufgabe, die Cäsar-Verschlüsselung in Java zu implementieren.
Dies habe ich auch (hoffentlich) geschafft. Also ich habe ein paar Teststrings ver- und entschlüsselt und verglichen.

Hier ist mal der Quellcode:

import java.util.Scanner;

public class CaesarCrypt {

    private char alphabet[];
   
    public CaesarCrypt() {
        initAlphabet();
    }
   
    public String encode(String text) {
        StringBuilder sb = new StringBuilder();
        String newText = text.trim();
        char letter[] = newText.toLowerCase().toCharArray();
        int current = 0;

        for(int k = 0; k < letter.length; k++) {
            for(int i = 0; i < alphabet.length; i++) {
                if(letter[current] == alphabet[i]) {
                    if((i + 3) >= alphabet.length)
                        sb.append(alphabet[(int) Math.abs((alphabet.length - i)-3)]);
                    else
                        sb.append(alphabet[i+3]);
                }
                if(i == alphabet.length - 1)
                    current++;
            }
        }
        return sb.toString();
    }
   
    public String decode(String text) {
        StringBuilder sb = new StringBuilder();
        String newText = text.trim();
        char letter[] = newText.toLowerCase().toCharArray();
        int current = 0;

        for(int k = 0; k < letter.length; k++) {
            for(int i = 0; i < alphabet.length; i++) {
                if(letter[current] == alphabet[i]) {
                    if((i - 3) < 0)
                        sb.append(alphabet[(alphabet.length + i) - 3]);
                    else
                        sb.append(alphabet[i-3]);
                }
                if(i == alphabet.length - 1)
                    current++;
            }
        }
        return sb.toString();
    }
   
    public void initAlphabet() {
        alphabet = new char[26];
        char c = 'a';
        for(int i = 0; i < alphabet.length; i++) {
            alphabet[i] = c;
            c++;
        }
    }
   
    public static void main(String[] args) {
        int in;
        Scanner s = new Scanner(System.in);
        CaesarCrypt cc = new CaesarCrypt();
       
        System.out.println("###########################");
        System.out.println("#                         #");
        System.out.println("# Caesar-Verschluesselung #");
        System.out.println("#                         #");
        System.out.println("###########################");
        System.out.println("#                         #");
        System.out.println("#                         #");
        System.out.println("# 1 - Verschluesseln      #");
        System.out.println("# 2 - Entschluesseln      #");
        System.out.println("#                         #");
        System.out.println("#                         #");
        System.out.println("###########################");
       
        in = s.nextInt();
       
        if(in == 1) {
            System.out.println();
            System.out.print("Text: ");
            System.out.println("Verschluesselt: " + cc.encode(s.next()));
        } else if(in == 2) {
            System.out.println();
            System.out.print("Text: ");
            System.out.println("Entschluesselt: " + cc.decode(s.next()));
        }
    }
}

Ich finde irgendwie, dass ich das unsauber gelöst habe. Vorallem die for-Schleifen-Verschachtelung finde ich sehr unübersichtlich.
Nun zu meiner Frage:
Kann man es übersichtlicher, "sauberer" lösen?
Und zweitrangig: Ist mein Programmierstil so in Ordnung?

Ich bin für jede Antwort sehr dankbar.

 

[SlaterB]

die innere for-Schleife ist unnötig,

besonders interessant versteckt ist dabei
> if(i == alphabet.length - 1) current++;
das passiert genau einmal pro innere Schleife über dem alphabet und zwar beim letzten Zeichen,
genausogut könntest du also ganz einfach
current++;
hinter der inneren Schleife schreibe, hätte den gleichen Effekt,

current wird aber gar nicht benötigt, das k der äußeren for-Schleife hat doch immer den gleichen Wert,
> for(int k = 0; k < letter.length; k++) {
ist eine Schleife über alle Buchstaben im letter-Array, dann ist doch letter[k] der aktuelle Buchstabe, wozu noch current?

------

und wie gesagt die innere Schleife, die kann weg, zumindest aus dem Hauptcode,
du brauchst die Position von letter[k] im alphabet-Array, ok,

egal wie man das anstellt, auf jeden Fall besser in einer Submethode, also

int index = findIndex(letter[k]);
if .. {
add alpabet[index +3]
} else {
add alpabet[index Math.abs usw]
}

dann siehts gleich aufgeräumter aus,
wie auch immer findIndex() funktioniert, das muss nur einmal programmiert werden und belastet encode/ decode nicht so sehr wenn es woanders steht

wie findIndex vorgeht ist variabel, statt ner Schleife könnte man die Position auch ausrechnen,
da 'a' den ASCII-Code 97 hat, 'b' 99 usw, musst du für ein Zeichen aus letter nur minus 'a' rechnen,
dann passt es,

char c = 'f';
int position = c - 'a'; // position = 5 oder so

Probleme bekommst du, wenn jemand ß oder Umlaute eintippt, dann lieber noch prüfen, ob die errechnete Position noch in den Grenzen des Arrays liegt,
durchaus also etwas kompliziert, musst du auch nicht machen

---------

ganz wichtig wäre noch, die 3, den wichtigen Verschiebungsschlüssel, in eine Variable auszulagern,
wenn du statt 3 Buchstaben lieber um 4 verschieben willst, müßtest du derzeit 6 verschiedene Codestellen ändern,
jede vergessene Stelle wäre fatal,
wenn du aber dagegen
private int key = 3;
zu
private int key = 4;
änderst, ist das leicht und praktisch nicht falsch zu machen