Methoden Code Quality und Stil

[unseen]

Guten Abend Leute,

ich bin noch ziemlicher Java Anfänger und würde mir gerne von Anfang an guten Stil angewöhnen. Leider kenne ich niemanden der sich meinen Code anschauen könnte und mir Tipps und Empfehlungen geben kann. Ich fände es echt mega wenn einer von euch sich kurz Zeit dafür nehmen würde.

Ich habe eine Methode geschrieben die aus einem String z.b. "Hallo Java Freunde" "H3e J2a F5e" macht, falls die String mehr als 3 Zeichen haben. Die Zahlen stehen für die Anzahl der weggelassen Zeichen.

Die Methode funktioniert auch einwandfrei ich würde nur gerne wissen ob es guter Stil ist oder ob das auch noch besser und/oder viel einfacher geht

public String abbreviate(String string){
       
        String[] eachWord = new String[string.length()];
        eachWord = string.split(" |\\-");
        int laengeSubstring = 0;
       
        for(int i = 0; i < eachWord.length; i++){
           
            if(eachWord[i].length() > 3){
               
                laengeSubstring = Integer.valueOf(eachWord[i].substring(1, eachWord[i].length() - 1).length());
                String laengeSubstringToString = "" + laengeSubstring;
           
                eachWord[i] = eachWord[i].charAt(0) + laengeSubstringToString + eachWord[i].charAt(eachWord[i].length() - 1);
           
            }
        }
       
        StringBuilder sb = new StringBuilder();
       
        for(String z : eachWord){
            sb.append(z);
            sb.append(" ");
        }

        String output = sb.toString();
       
        return output;
    }

 

[Thallius]

Also das new String() kannst du dir sparen, da das Split ein fertiges Array zurück liefert.
Ich würde mir von Anfang an angewöhnen alles in englisch zu schreiben. Wenn du später für irgendjemanden arbeitest ist das eh Voraussetzung. Dieser Mischmasch aus deutsch und englisch geht jedenfalls gar nicht.
Die Variable eachWord finde ich schlecht gewählt. Hierbei handelt es sich ja um ein Array von Wörtern. Nenn es also besser wordArray oder meinetwegen wordList.
Warum berechnest du deine laengeSubstring nicht einfach aus String.length() - 2?
Durch die Wörter würde ich mit einem Iterator iterieren. Dadurch erhältst du eine Variable z.b. Word was das ganze viel leserlicher macht als immer woerter benutzen zu müssen. Also for(String Word : wordArray)

Gruß

Claus

 

[JStein52]

for(String Word : wordArray)

na na na

for(String word : wordArray)

Doch sicher so !

 

[Thallius]

na na na

for(String word : wordArray)

Doch sicher so !

Naja ist halt mühsam auf dem iPad. Das macht immer was es will mit der Groß-Kleinschreibung...

 

[mrBrown]

    public String abbreviate(String string){
   
        String[] eachWord = new String[string.length()];
        eachWord = string.split(" |\\-");
        int laengeSubstring = 0;
   
        for(int i = 0; i < eachWord.length; i++){
       
            if(eachWord[i].length() > 3){
           
                laengeSubstring = Integer.valueOf(eachWord[i].substring(1, eachWord[i].length() - 1).length());
                String laengeSubstringToString = "" + laengeSubstring;
       
                eachWord[i] = eachWord[i].charAt(0) + laengeSubstringToString + eachWord[i].charAt(eachWord[i].length() - 1);
       
            }
        }
   
        StringBuilder sb = new StringBuilder();
   
        for(String z : eachWord){
            sb.append(z);
            sb.append(" ");
        }

        String output = sb.toString();
   
        return output;
    }

Ich machs einfach mal ausführlich 😉


Die Methode macht aktuell zwei Dinge - den Text trennen und über alle Wörter iterieren, und dann für jedes Wort Länge berechnen und ersetzten.

Den Schleifeninhalt kann man einfach auslagern und die dann überflüssigen Variablen entfernen.

Spoiler: Code

public String abbreviate(String string) {

        String[] eachWord = new String[string.length()];
        eachWord = string.split(" |\\-");

        for (int i = 0; i < eachWord.length; i++) {
            eachWord[i] = abbreviateWord(eachWord[i]);
        }

        StringBuilder sb = new StringBuilder();

        for (String z : eachWord) {
            sb.append(z);
            sb.append(" ");
        }

        String output = sb.toString();

        return output;
    }

    private String abbreviateWord(String word) {
        if (word.length() > 3) {

            int laengeSubstring = Integer.valueOf(word.substring(1, word.length() - 1).length());
            String laengeSubstringToString = "" + laengeSubstring;

            word = word.charAt(0) + laengeSubstringToString + word.charAt(word.length() - 1);

        }
        return word;
    }

In abbreviateWord kann man die Länge des Substrings deutlich einfacher berechnen - es ist ja einfach die Länge des Strings minus zwei. (und gleichzeitig die Benennung vereinheitlichen)

Spoiler: Code

    private String abbreviateWord(String word) {
        if (word.length() > 3) {
            String lengthSubstring = word.length() - 2 + "";

            word = word.charAt(0) + lengthSubstring + word.charAt(word.length() - 1);
        }
        return word;
    }

In abbreviate würde ich das Array words statt eachWord nennen, und es gleichzeitig direkt initialisieren.
Zusätzlich würde ich für die abgekürzten Wörter eine neue Liste nehmen, und nicht das Array verändern.
Die erste Schleife kann man dann auch als for-each schreiben.

Spoiler: Code

        String[] words = string.split(" |\\-");

        List<String> abbreviatedWords = new ArrayList<>();

        for (String word : words) {
            abbreviatedWords.add(abbreviateWord(word));
        }

Für das zusammenfügen der Wörter würde ich auf die händische Iteration (welche am Ende auch ein Leerzeichen zu viel anfügt) verzichten, und stattdessen String.join nutzen, was eine Liste von Strings und ein Trennzeichen bekommt und die Strings entsprechend aneinanderfügt.

Spoiler: Code

public String abbreviate(String string) {

    String[] words = string.split(" |\\-");

    List<String> abbreviatedWords = new ArrayList<>();

    for (String word : words) {
        abbreviatedWords.add(abbreviateWord(word));
    }

    return String.join(" ", abbreviatedWords);
}

private String abbreviateWord(String word) {
    if (word.length() > 3) {
        String lengthSubstring = word.length() - 2 + "";

        word = word.charAt(0) + lengthSubstring + word.charAt(word.length() - 1);
    }
    return word;
}

Und wenn man dann noch Stream nutzt: 😉

Spoiler: Code

    public String abbreviate(String string) {
        return Arrays.stream(string.split(" |\\-"))
                .map(this::abbreviateWord)
                .collect(Collectors.joining(" "));
    }

 

[unseen]

Wow danke für die ausführliche Antwort, damit hätte ich nicht gerechnet 🙂. Listen und Streams habe ich mir leider noch nicht angeschaut, aber Listen werden definitiv mein nächstes Thema.

Beeindruckend wie einfach ein kleines Programm aussehen kann.