code kürzen

Diskutiere code kürzen im Java Basics - Anfänger-Themen Bereich.
D

Daniel710

gibt es eine möglichkeit den code zu kürzen oder zu vereinfachen?

Java:
public void readAndStoreTags(String path) {
        Map<String, Object> tag_map = TagReader.readTags(path);
        for (String key : tag_map.keySet()) {
            if (key == "title") {
                if (tag_map.get(key).toString() != "" || tag_map.get(key).toString() != null) {
                    title = tag_map.get(key).toString().trim();
                }
            } else if (key == "author") {
                if (tag_map.get(key).toString() != "" || tag_map.get(key).toString() != null) {
                    author = tag_map.get(key).toString().trim();
                }  
            } else if (key == "album") {
                if (tag_map.get(key).toString() != "" || tag_map.get(key).toString() != null) {
                    album = tag_map.get(key).toString().trim();
                }
            } else if (key == "duration") {
                if ((long) tag_map.get(key) > 0) {
                    duration = (long)tag_map.get(key);  
                }
            }
        }
    }
 
Zuletzt bearbeitet:
L

LimDul

Sinnvoll wäre es im ersten Schritt die Fehler zu beseitigen :) Wenn du ihn in CODE-Tags setzt, wird er lesbar.
Strings vergleicht man nicht mit == sondern mit equals. Bei duration wird dir eine Nullpointer Exception fliegen, wenn der Wert in der map null ist.

Du kannst allerdings Strings in einem switch-case verwenden, dann sparst du dir die if-else cascade und kannst mit case-Statements arbeiten, das ist dann übersichtlicher. Die Abfrage ob der Value in der Map null ist, kannst du unter Umständen auch am Anfang machen und bei null einfach continue aufrufen. Oder man könnte - sofern das fachlich ok wäre - default Werte setzen, wenn der Wert null bzw. ein Leer String ist.
 
mrBrown

mrBrown

Die Schleife ist überflüssig, da du ja die Tags kennst, der Vergleich mit == klappt nicht und der Check auf null ist in der Form sinnlos, du würdest einfach ein NPE bekommen, wenn irgendwas null ist.


Java:
public void readAndStoreTags(String path) {
        Map<String, Object> tag_map = TagReader.readTags(path);
                if (tag_map.get("title")) != null && !tag_map.get("title").toString().trim().isEmpty()) {
                    title = tag_map.get("title").toString().trim();
                }
               if (tag_map.get("author")) != null && !tag_map.get("author").toString().trim().isEmpty()) {
                    title = tag_map.get("author").toString().trim();
                }
               if (tag_map.get("album")) != null && !tag_map.get("album").toString().trim().isEmpty()) {
                    title = tag_map.get("album").toString().trim();
                }
               if (tag_map.get("duration")) != null) {
                    title = (long)tag_map.get("duration");  
                }
        }
    }
 
MoxxiManagarm

MoxxiManagarm

Ich hätte es in etwa so probiert:
Java:
public void readAndStoreTags(String path) {
    Map<String, Object> tag_map = TagReader.readTags(path);

    title = tag_map.getOrDefault("title", "").toString().trim();
    author = tag_map.getOrDefault("author", "").toString().trim();
    album = tag_map.getOrDefault("album", "").toString().trim();
    duration = (long)tag_map.getOrDefault("duration", 0);
}
 
W

White_Fox

Wenn ich mich recht entsinne, kann man Strings (im Gegensatz zu anderen Objekten) auch als Case-Separator in einem Switch-Statement verwenden.
 
mrBrown

mrBrown

Ich hätte es in etwa so probiert:
Java:
public void readAndStoreTags(String path) {
    Map<String, Object> tag_map = TagReader.readTags(path);

    title = tag_map.getOrDefault("title", "").toString().trim();
    author = tag_map.getOrDefault("author", "").toString().trim();
    album = tag_map.getOrDefault("album", "").toString().trim();
    duration = (long)tag_map.getOrDefault("duration", 0);
}
Wobei die Werte dann im Zweifel auf 0/"" gesetzt werden, Je nachdem wie das benutzt wird kann das ein Problem sein
 
D

Daniel710

Ich hätte es in etwa so probiert:
Java:
public void readAndStoreTags(String path) {
    Map<String, Object> tag_map = TagReader.readTags(path);

    title = tag_map.getOrDefault("title", "").toString().trim();
    author = tag_map.getOrDefault("author", "").toString().trim();
    album = tag_map.getOrDefault("album", "").toString().trim();
    duration = (long)tag_map.getOrDefault("duration", 0);
}
ich habe das mal in mein programm eingefügt und eine testdatei ausgeführt. in dem geänderten programmteil läuft alles gut, aber jetzt bekomme ich an einer anderen stelle einen fehler, da mir dein code aber besser gefällt würde ich deins gerne übernehmen. der test meldet in folgendem abschnitt einen fehler, hast du eine idee wie das zusammenhängt?

Java:
/**
     * Convert string
     *
     * @return string
     */
public String toString() {
        String auxiliaryString = super.toString();
        String auxiliaryDuration = getFormattedDuration();
        
        if (album != "" && album != null) {
            return (auxiliaryString + " - " + album + " - " + auxiliaryDuration);
        } else {
            return (auxiliaryString + " - " + auxiliaryDuration);
        }
    }
 
L

LimDul

Strings vergleicht man nicht mit == bzw !=. Das ist fehleranfällig. Und der Vergleich auf != null gehört an die erste Stelle in der If Bedingung.

Java:
        if (album != null && !album.equals("")) {
bzw.
Java:
        if (album != null && !"".equals(album)) {
 
D

Daniel710

Strings vergleicht man nicht mit == bzw !=. Das ist fehleranfällig. Und der Vergleich auf != null gehört an die erste Stelle in der If Bedingung.

Java:
        if (album != null && !album.equals("")) {
bzw.
Java:
        if (album != null && !"".equals(album)) {
okay :D habs jetzt geändert, funktioniert aber trotzdem nicht :( ich weiß aber auch nicht wo da der zusammenhang zu dem oben ist
 
L

LimDul

Inwefern bekommst du den einen Fehler? Compile Fehler? Falsche Ausgabe? Was wäre den richtig?

So ohne Glaskugel ist es schwer :)
 
D

Daniel710

ich weiß es auch nicht, wir haben eine testdatei bekommen und die zeigt an dass in der zuletzt eingestellten methode ein fehler ist. ihr habt mir aber schon viel weitergeholfen, wenn ihr jetzt auch keinen fehler mehr seht ists egal. vielen dank für eure antworten und anregungen!
 
D

Daniel710

Screenshot_1.png
es gibt quasi keine fehlermeldung, es zeigt nur an in welcher methode noch was schief läuft
 
mrBrown

mrBrown

es gibt quasi keine fehlermeldung, es zeigt nur an in welcher methode noch was schief läuft
Guck dir mal die Ausgabe an, da sollte ziemlich genau stehen was nicht korrekt ist.

Falls du die Zeile nicht selbst markiert hast: Deine toString schlägt fehl, wenn kein Album angegeben ist.
 
QU3LLC0D3

QU3LLC0D3

Strings vergleicht man nicht mit == bzw !=. Das ist fehleranfällig. Und der Vergleich auf != null gehört an die erste Stelle in der If Bedingung.

Java:
        if (album != null && !album.equals("")) {
bzw.
Java:
        if (album != null && !"".equals(album)) {
Was spricht denn gegen die Verwendung von StringUtils? Da kann man doch gleich alles in einem abfrühstücken...

Code:
if (StringUtils.isNotEmpty(album)) {
  // do something
}
isNotEmpty prüft auf null und Länge = 0, isNotBlank prüft auf null auf Länge = 0 und auf Whitespaces ...

Des Weiteren nutze ich immer gern bei der Zuweisung von Strings die "trimToEmpty()" Methode, ebenfalls aus den StringUtils. Dies sichert mir zu, dass der String niemals null sein kann.
 
mihe7

mihe7

Doch, man führt hier Abhängigkeiten zu externen Bibliotheken ein und das für etwas völlig triviales.
Java:
public abstract class Strings {
    public static String isNotBlank(String value) {
        return value != null && !value.isBlank();
    }

    public static String trimToEmpty(String value) {
        return value == null ? "" : value.trim();
    }
}
 
Thema: 

code kürzen

Passende Stellenanzeigen aus deiner Region:
Anzeige

Neue Themen

Anzeige

Anzeige
Oben