Row-Klasse Implementieren

Euler123

Mitglied
Hallo zusammen,

Ich habe eine Aufgabe gegeben, wo ich unter anderem die Klasse Row implementieren soll:

Die Klasse Row stellt einen Eintrag (eine Zeile bzw. ein Tupel) einer Tabelle dar und hat die folgenden Instanzvariablen:
  • dimension: Die maximale Anzahl an Attributen (Spalten) eines Eintrags.
  • values: Die Werte der jeweiligen Attribute als double-Array.
Die Dimension wird mittels Konstruktor gesetzt. Das Werte-Array soll dimension viele double-Werte enthalten, wobei alle Werte mit einem speziellen Wert Double.NaN initialisiert werden. NaN ist eine Abkürzung für Not a Number (in diesem Fall vom Datentyp double) und wird in unserem Beispiel verwendet um zu signalisieren, dass ein Wert in einer Zeile nicht existiert.

Java:
public class Row {
    
    private int dimension;
    private double[] values;
    
    /**
     * Creates a new row.
     *
     * All entries are initially set to `Double.NaN`.
     *
     * @param dimension The number of elements of that this row contains.
     */
    public Row(int dimension) {
        this.dimension = dimension;
        this.values = new double[dimension];
        for (int i = 0; i < dimension; i++) {
            this.values[i] = Double.NaN;
        }
    }

    /**
     * Get the dimension of this row.
     */
    public int getDimension() {
        return dimension;
    }

    /**
     * Get the values stored in this row.
     */
    public double[] getValues() {
        return values;
    }

    /**
     * Sets the values of this row.
     *
     * This replaces the entire array, not just its elements, and updates
     * the dimension of this row (according to the length of the given
     * `double` array).
     *
     * @param values The `double` array that replaces the `double` array
     *   of the current row.
     */
    public void setValues(double[] values) {
        this.values = values.clone();
        this.dimension = values.length;
    }
    
    
    /**
     * Get a value in this row.
     *
     * @param index The index of the value to get.
     * @return If the index is invalid (that is, negative or our of bounds),
     *   `Double.NaN`; otherwise, the value at that index (which may also be
     *   `Double.NaN`).
     */
    
    public double getValue(int index) {
        if (index < 0 || index >= dimension) {
            return Double.NaN;
        }
        return values[index];
    }
    /**
     * Set a value in this row.
     *
     * @param index The index of the value to set.
     * @param value The value to set the element at that index to.
     * @return `false` if the index is invalid (that is, negative or our of
     *   bounds), or if the element at that index already has the value `value`;
     *   `true` otherwise.
     */
    public boolean setValue(int index, double value) {
       if (index < 0 || index >= dimension || Double.isNaN(value)) {
            return false;
        }
        if (values[index] == value) {
            return false;
        }
        values[index] = value;
        return true;
    }

    /**
     * Get the average of all values in this row.
     *
     * @return The arithmetic mean of all non-NaN entries in this row; note that
     * NaN entries are to be excluded entirely; e.g. if a row has 10 entries;
     * only three of which—`a`,`b`, `c`—are not NaN, then this should return
     * `(a + b + c) / 3`.
     */
    public double getAverage() {
        double sum = 0.0;
        int count = 0;

        for (double value : values) {
            if (!Double.isNaN(value)) {
                sum += value;
                count++;
            }
        }

        if (count == 0) {
            return Double.NaN;
        }

        return sum / count;
    }

    /**
     * Convert a row to a string.
     */
    @Override
    public String toString() {
        StringBuilder s = new StringBuilder();
        double[] vs = getValues();

        s.append("|");

        for (double v : vs) {
            s.append(String.format(" %5.3f |", v));
        }

        s.append(String.format(" %5.3f |", getAverage()));

        return s.toString();
    }
}

Dabei bekomme ich jetzt folgende Fehlermeldung:
1703623251882.png
Bei dem setValues und getValue scheint es also Probleme zu geben - ich habe jetzt schon ziemlich viel herumprobiert, komme aber nicht wirklich auf meinen fehler. Kann mir da vielleicht jemend einen Tipp geben?

Schöne Weihnachtsfeiertage,
LG Euler
 

KonradN

Super-Moderator
Mitarbeiter
Evtl. prüft der Test von setValues(), ob danach getValues() das gleiche Array zurück gibt. Du könntest also einmal ausprobieren, den Parameter direkt zu setzen statt einem Clone.

Den zweiten Fehler kann ich gerade nicht erklären. Die Methode sieht erst einmal ok aus auf den ersten Blick.
 

Marinek

Bekanntes Mitglied
Zeile 79 und 80 sieht überflüssig aus.
Also in dieser Aufgabe geht es um das Lernen von Test Driven Development (TDD).

Gemäß der Spezifikation soll die Methode false zurück geben, wenn der Wert da schon gesetzt worden ist.

to.
* @Return false if the index is invalid (that is, negative or our of
* bounds), or if the element at that index already has the value value;
*

Ich denke das ist hiermit eindeutig.

Es ist im übrigen ganz schlechter Stil out of topic einen Satz zu droppen, der zudem offensichtlich falsch ist, und diesen nicht näher zu erläutern.

Ich denke wir haben hier ansonsten einen Thread „was hört man für Musik“. Vielleicht ist das mehr so deine fachliche domaine?

@Euler123

Du solltest vorher prüfen, ob das Array null ist. Sonst hättest Du an der Stelle eine Pot. Null Pointer Exception.

Ansonsten bin ich ganz bei Konrad.
 
Zuletzt bearbeitet:

Marinek

Bekanntes Mitglied
Das zeigt deine gute Kinderstube vermutlich, die du im CPP Forum angesprochen hast? 🤣
 
Zuletzt bearbeitet:

Euler123

Mitglied
Hallo zusammen und danke für die Vorschläge :)
Evtl. prüft der Test von setValues(), ob danach getValues() das gleiche Array zurück gibt. Du könntest also einmal ausprobieren, den Parameter direkt zu setzen statt einem Clone.
Dass hat leider schon mal nichts geändert.

Dann habe ich es auch noch so probiert (hat aber auch nichts genützt:
Java:
public void setValues(double[] values) {
        if (values.length != dimension) {
            return;
        }
        for (int i = 0; i < dimension; i++) {
            this.values[i] = values[i];
        }
    }

Durch das clone gibt er schon das gleiche Array zurück. Geprüft wird aber vermutlich auf dasselbe Array. Kleiner aber feiner Unterschied ;-)
Das klingt erstmal plausibel, aber wie kann ich das erreichen (da fehlt mir jetzt die Idee??)
LG Euler
 

Marinek

Bekanntes Mitglied
Hi,

Die Idee von @Konrad geht dahin, dass man das so macht:


Java:
public void setValues(double[] values) {
        this.values = values;
        this.dimension = values.length;
    }

Also nie clone verwenden. Auch nicht bei der Rückgabe.
 

KonradN

Super-Moderator
Mitarbeiter
Das klingt erstmal plausibel, aber wie kann ich das erreichen (da fehlt mir jetzt die Idee??)
Also der Hinweis bezog sich auf meine Aussage, die das korrekte meinte aber ich habe mich schlicht falsch ausgedrückt. Halt ein Fehler, der Umgangssprachlich leider öfters mal vorkommt (und was ich als peinlich empfunden habe, denn ich hatte bisher gemeint, dass ich auf sowas auch achten würde ...)

Also ganz wichtig: Du bekommst eine Referenz auf ein Array übergeben und genau diese Referenz speicherst Du ab. Und wenn danach gefragt wird, dann wird auch genau diese Referenz zurück gegeben.

Das kann evtl. dazu führen, dass der Unit Test durchläuft.

Es gibt aber sehr wohl eine gewisse Praxis, dass man den inneren Zustand einer Klasse nicht nach außen gibt. Hintergrund ist, dass so der innere Zustand von außerhalb geändert werden kann und das bricht die Kapselung.

Die Diskussion, ob oder wann man sowas machen sollte, fange ich jetzt aber nicht an. Man findet es durchaus öfters, dass hier die Kapselung unsauber ist. Ein Beispiel, das mir direkt einfällt wäre JavaFX mit dem typischen getChildren().add(...) ...
 

Euler123

Mitglied
Die erste Fehlermeldung habe ich jetzt doch gelöst bekommen (vielen Dank - auch für die Erklärung dazu) - es lag jetzt wirklich an dem clone. Zur zweiten Fehlermeldung bin ich immer noch ratlos??

PS: @KonradN: Das man sich mal falsch Ausdrückt bzw. Interpretationsfreudig passiert ja jedem mal - dafür braucht man sich nicht rechtfertigen, ist ja quasi legitim :) :)
 

Euler123

Mitglied
Der zweiter Fehler hat sich jetzt doch auch gleich gelöst
Java:
public double getValue(int index) {
         if (index < 0 || index >= values.length) {
            return Double.NaN;
        }
        return values[index];
    }

Der Fehler hat sich dadurch gelöst, dass ich dimension durch values.lenght ersetzt habe
 

Marinek

Bekanntes Mitglied
Das würde aber bedeuten, dass die Eigenschaft "dimension" irgendwo bei dir falsch gesetzt wird. Denn im Konstruktor machst du es ja richtig. Und dort ist dimension == values.length.
 

KonradN

Super-Moderator
Mitarbeiter
Genau, und auch in der setValues Methode setzt du dimension so dass dimension immer == values.length sein müsste.

Also bitte genau prüfen:
  • Immer, wenn values zugewiesen wird, dann sollte dimension auf values.length gesetzt werden.
  • dimension sollte ansonsten nie verändert werden

Ansonsten ist das übrigens ein Punkt, der mir etwas aufgestoßen ist. dimension zu speichern ist sozusagen doppelt. Daten Doppelung vermeidet man in der Regel. Es würde also Sinn machen, die Instanzvariable dimension zu löschen und getDimension() sollte einfach values.length zurück geben.
Aber: Die Aufgabe hat dies explizit gefodert, daher ist das hier nicht möglich!
 

KonradN

Super-Moderator
Mitarbeiter
was ein Anti-Pattern ist.
Was wieder eine typische Tobias Aussage ist. "Anti-Pattern" ohne jede Begründung ist einfach ohne jeden inhaltlichen Wert. Zumal ich in meinem Post, bei dem ich diesen Punkt erwähnt habe, bereits mehr Begründung genannt habe. Und komplett "Anti-Pattern" scheint nicht zu stimmen, sonst würde man sowas nicht auch im Framework finden :)
 

KonradN

Super-Moderator
Mitarbeiter
Aber wo wir jetzt im Thread schon dieses Thema (Rückgabe eines Arrays) haben, möchte ich das doch etwas ausführlicher behandeln.

Die Kernproblematik habe ich schon genannt: Wir wollen die Daten in der Klasse kapseln, d.h. diese sind nicht direkt von außen veränderbar.

Sobald ich diesen nun aber in Form einer Referenz nach außen gebe, kann man den auch außerhalb der Klasse verändern. Damit breche ich also die Kapselung.

"Anti-Pattern" ist etwas, das besagt: Sowas macht man generell nicht. Das ist also ein sehr hartes Urteil, das ich nicht teile. Es gibt genug Beispiele, bei denen sowas ganz bewusst gemacht wurde. Ein guter Grund kann z.B, sein, dass direkte Zugriffe aus z.B. Geschwindigkeitsgründen explizit erwünscht sind.

Daher ist die Aussage mit "Anti-Pattern" aus meiner Sicht zu hart. Aber natürlich hat es einen waren Kern und es gibt entsprechende Regeln.

Bei PMD wäre da z.B. die Regel MethodReturnsInternalArray:
Best Practices | PMD Source Code Analyzer

Inhaltlich hat Tobias als nicht wirklich etwas falsches gesagt - nur eben ist diese Art von Antwort schlicht nicht hilfreich. Ohne Erklärungen oder weiterführende Links kann man sich sowas schlicht sparen. Und man muss da nicht einmal viel schreiben - es gibt genug Erklärungen. PMD habe ich gebracht, aber auch andere (statische) Code Analyse Tools werden ähnliche Regeln haben, die dann dokumentiert sind mit Erklärungen und weiterführenden Links. Und eine Suche nach "PMD MethodeReturnsInternalArray" wird auch bestimmt viele Treffer mit Diskussionen bringen. Es wäre also einfach, mit ganz wenig Worten auch entsprechende Links / Hinweise zu geben.
 

Marinek

Bekanntes Mitglied
Inhaltlich hat Tobias als nicht wirklich etwas falsches gesagt - nur eben ist diese Art von Antwort schlicht nicht hilfreich. Ohne Erklärungen oder weiterführende Links kann man sich sowas schlicht sparen. Und man muss da nicht einmal viel schreiben - es gibt genug Erklärungen. PMD habe ich gebracht, aber auch andere (statische) Code Analyse Tools werden ähnliche Regeln haben, die dann dokumentiert sind mit Erklärungen und weiterführenden Links. Und eine Suche nach "PMD MethodeReturnsInternalArray" wird auch bestimmt viele Treffer mit Diskussionen bringen. Es wäre also einfach, mit ganz wenig Worten auch entsprechende Links / Hinweise zu geben.
Grundsätzlich ist das alles richtig und wichtig. Wenn ich später Multi Tier Architekturen baue und mit hunderten Entwicklern ein Code bearbeite... Aber ich wage es zu bezweifeln, dass in den Hausaufgaben gerade dies gefordert wird. Hier scheint es um simple TDD Anästze zu gehen.. Auch Punkte, die nicht explizit in der Spezifikation stehen, sind hier in den Tests scheinbar umgesetzt worden. Üblicherweise würde man hier in den Projekten einen Step Back machen und prüfen, ob die Spezifikation sinnvoll ist.

Von daher kann es ein Anti Pattern sein, aber..

  • Es ist hier schlicht over the Topic
  • Es ist hier scheinbar gewollt

So einen Einsatz Einwurf halte ich ehh für schwachsinn..

(Da hat man schnell reagiert ;))
 

Euler123

Mitglied
Das würde aber bedeuten, dass die Eigenschaft "dimension" irgendwo bei dir falsch gesetzt wird. Denn im Konstruktor machst du es ja richtig. Und dort ist dimension == values.length.
Warum das jetzt genau so funktioniert hat, weiß ich eh nicht genau? (ich hab da halt noch ein paar Sachen ausprobiert und das hat halt geklappt - in die Test an sich habe ich aber auch keinerlei Einsicht (Ich persönlich finde die Tests teils sehr kryptisch).
 

KonradN

Super-Moderator
Mitarbeiter
Warum das jetzt genau so funktioniert hat, weiß ich eh nicht genau? (ich hab da halt noch ein paar Sachen ausprobiert und das hat halt geklappt - in die Test an sich habe ich aber auch keinerlei Einsicht (Ich persönlich finde die Tests teils sehr kryptisch).
Du kannst uns einmal die ganze Klasse zeigen, dann können wir auch einmal schauen. Oder wenn Du die ganze Klasse nicht zeigen möchtest im Forum, dann könntest Du im Forum ein Gespräch mit mir starten und mir den Code dort zeigen. Dann wäre es nicht öffentlich.
 

Euler123

Mitglied
Du kannst uns einmal die ganze Klasse zeigen, dann können wir auch einmal schauen. Oder wenn Du die ganze Klasse nicht zeigen möchtest im Forum, dann könntest Du im Forum ein Gespräch mit mir starten und mir den Code dort zeigen. Dann wäre es nicht öffentlich.
Ja mach ich gern - also das ist jetzt die ganze Klasse Row, welche alle Test erfüllt hat:

Java:
public class Row {
    
    private int dimension;
    private double[] values;
    
    /**
     * Creates a new row.
     *
     * All entries are initially set to `Double.NaN`.
     *
     * @param dimension The number of elements of that this row contains.
     */
    public Row(int dimension) {
        this.dimension = dimension;
        this.values = new double[dimension];
        for (int i = 0; i < dimension; i++) {
            this.values[i] = Double.NaN;
        }
    }

    /**
     * Get the dimension of this row.
     */
    public int getDimension() {
        return dimension;
    }

    /**
     * Get the values stored in this row.
     */
    public double[] getValues() {
        return values;
    }

    /**
     * Sets the values of this row.
     *
     * This replaces the entire array, not just its elements, and updates
     * the dimension of this row (according to the length of the given
     * `double` array).
     *
     * @param values The `double` array that replaces the `double` array
     *   of the current row.
     */
    public void setValues(double[] values) {
        this.values = values;
        this.dimension = values.length;
    }
    
    /**
     * Get a value in this row.
     *
     * @param index The index of the value to get.
     * @return If the index is invalid (that is, negative or our of bounds),
     *   `Double.NaN`; otherwise, the value at that index (which may also be
     *   `Double.NaN`).
     */
    
    public double getValue(int index) {
         if (index < 0 || index >= values.length) {
            return Double.NaN;
        }
        return values[index];
    }
    /**
     * Set a value in this row.
     *
     * @param index The index of the value to set.
     * @param value The value to set the element at that index to.
     * @return `false` if the index is invalid (that is, negative or our of
     *   bounds), or if the element at that index already has the value `value`;
     *   `true` otherwise.
     */
    public boolean setValue(int index, double value) {
       if (index < 0 || index >= dimension || Double.isNaN(value)) {
            return false;
        }
        if (values[index] == value) {
            return false;
        }
        values[index] = value;
        return true;
    }

    /**
     * Get the average of all values in this row.
     *
     * @return The arithmetic mean of all non-NaN entries in this row; note that
     * NaN entries are to be excluded entirely; e.g. if a row has 10 entries;
     * only three of which—`a`,`b`, `c`—are not NaN, then this should return
     * `(a + b + c) / 3`.
     */
    public double getAverage() {
        double sum = 0.0;
        int count = 0;

        for (double value : values) {
            if (!Double.isNaN(value)) {
                sum += value;
                count++;
            }
        }

        if (count == 0) {
            return Double.NaN;
        }

        return sum / count;
    }

    /**
     * Convert a row to a string.
     */
    @Override
    public String toString() {
        StringBuilder s = new StringBuilder();
        double[] vs = getValues();

        s.append("|");

        for (double v : vs) {
            s.append(String.format(" %5.3f |", v));
        }

        s.append(String.format(" %5.3f |", getAverage()));

        return s.toString();
    }
}
 

KonradN

Super-Moderator
Mitarbeiter
Also ich sehe da jetzt so auf Anhieb keinen Grund, wieso dimension falsch sein könnte. Daher wäre meine Erwartung, dass es auch mit dimension statt values.length funktionieren müsste.

Was mir auffällt, ist aber in setValue die Prüfung auf || Double.isNaN(value) - das ist etwas, das nicht im JavaDoc steht. Laut JavaDoc sollte auch ein NaN gesetzt werden können. Da also ggf. die Anforderung noch einmal prüfen und das ggf. herausnehmen.
 

Neue Themen


Oben