code kürzen

[Gelöschtes Mitglied 62813]

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

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);  
                }
            }
        }
    }

 

[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]

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.

Spoiler

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]

Ich hätte es in etwa so probiert:

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);
}

 

[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]

Ich hätte es in etwa so probiert:

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

 

[Gelöschtes Mitglied 62813]

Ich hätte es in etwa so probiert:

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?

/**
     * 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);
        }
    }

 

[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.

        if (album != null && !album.equals("")) {

bzw.

        if (album != null && !"".equals(album)) {

 

[Gelöschtes Mitglied 62813]

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.

        if (album != null && !album.equals("")) {

bzw.

        if (album != null && !"".equals(album)) {

okay habs jetzt geändert, funktioniert aber trotzdem nicht ich weiß aber auch nicht wo da der zusammenhang zu dem oben ist

 

[LimDul]

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

So ohne Glaskugel ist es schwer

 

[Gelöschtes Mitglied 62813]

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!

 

[LimDul]

Was ist denn die genaue Ausgabe der Fehlermeldung?

 

[Gelöschtes Mitglied 62813]

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

 

[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]

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.

        if (album != null && !album.equals("")) {

bzw.

        if (album != null && !"".equals(album)) {

Was spricht denn gegen die Verwendung von StringUtils? Da kann man doch gleich alles in einem abfrühstücken...

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.

 

[LimDul]

Was spricht denn gegen die Verwendung von StringUtils? Da kann man doch gleich alles in einem abfrühstücken...

Das es sich hier vermutlich um eine Uni-Aufgabe oder so handelt und man da nicht umbedingt apache-commons drauf werfen kann Ansonsten eigentlich nichts.

 

[mihe7]

Doch, man führt hier Abhängigkeiten zu externen Bibliotheken ein und das für etwas völlig triviales.

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();
    }
}