Wie viele Parameter sollte ein Objekt haben

[Rudolf]

Hallo,

ich habe noch immer für mich keine zufriedenstellende Lösung gefunden.

Es gibt Objekte, die haben mehrere Abhängigkeiten, die sie verarbeiten, und alle Abhängigkeiten sind Pflichtangaben. Wenn ich sie also mit dem Beans Pattern umsetze, dann ist das Objekt anfangs in einem unstabilen Zustand und immutable Objekte kann ich damit auch nicht machen. Mit normalen Objektkonstruktorn wird der Code schnell unübersichtlich und das Builder Pattern fällt auch weg, da alle Angaben Pflichtangaben und keine Optionalangaben sind.

Kennt jemand eine andere Lösung, oder weiß er wie solche Objekte in der Praxis generell behandelt werden?

Damit man sich das Ausmaß vorstellt. Es handelt sich um 9 Parameter.

 

[Tobse]

Da gibts nur eine Möglichkeit: Leg Standardwerte fest. Die müssen ja nicht gut genug sein, um perfekt zu Funktionieren, sondern nur um vllt zu verhindern, dass Exceptions fliegen.

 

[JohannisderKaeufer]

Es kann ja sein, daß die Objektstruktur Verbesserungspotential bietet.

public Person(String name, String prename, Calendar birthdate, Gender gender, String street, String housenumber, String zipCode, String City, Country country, String phoneNumber, String emailAdress)

public Person(String name, String prename, Calendar birthdate, Gender gender, Address address, ContactData contactData)

 

[Ghostfish]

Wie viele Parameter sollte ein Objekt haben?

Auch für Quellcode gelten in gewisser Weise Usability-Empfehlungen, auch Quellcode muss benutzbar sein. Mehr als sieben Objekte (allgemein gemeint, nicht Java-Objekte) kann ein Mensch schlecht gleichzeitig im Kopf behalten, und es wird unübersichtlich.

Alternativ könnte man größere Datenlisten entsprechend auch als Liste oder anderes Aggregationsobjekt übergeben, dann hat man nur noch einen Parameter.

 

[Rudolf]

Ich habe mir schon gedacht, dass es in die Richtung gehen soll.

Ich habe am Ende auch sowas gemacht. Ich habe Wrapperklassen erstellt, die 3 Objekte jeweils wrappen und alle benötigten Methoden delegieren. Ihr meint also, dass es keine bessere Lösung gibt?

Durch das delegieren wird es manchmal auch etwas unübersichtlich.

Da das beschreiben immer schwieriger wird, habe ich den Code mal hochgeladen:

https://github.com/rudolfschmidt/TicTacToe/blob/master/Tic Tac Toe/src/model/logic/GameLogic.java

So sieht der ganze Spass aus. Bitte um Feedback zum Design und ob es sich anders lösen lässt.

 

[ThreadPool]

[...] und das Builder Pattern fällt auch weg, da alle Angaben Pflichtangaben und keine Optionalangaben sind. [...]

Das Builder-Pattern kann auch mit Pflichtangaben verwendet werden, du abstrahierst von der eigentlichen Objekterzeugung mit new und kannst alle dir sinnvollen Bedingungen überprüfen und bei Nichteinhaltung eine Exception werfen.

Aber wenn sowieso alle Angaben Pflichtangaben sind kann man sie auch über den Konstruktor übergeben, und wenn es dir zu viele Parameter sind dann bleibt dir noch die Möglichkeit sie in ein sinnvolles Parameterobjekt auszulagern.

 

[JCODA]

Also, so leid mirs tut, aber ich würde das als 3x3-Array übergeben, klar, dann ist die Berechnung vielleicht ein bisschen aufwendiger, aber letztendlich wrappst du hier 3 mal 3 gleiche ObjektTypen.

Achso, dein FieldHandler heißt FieldHadler, oder sollte das so sein?

 

[bygones]

was mir am Design nicht sehr gefaellt ist dass es keine Abstraktionen gibt und sich Strukturen dem Anschein nach wiederholen.

Was ist der Unterschied zwischen FirstRowLogic, SecondRowLogic and ThirdRowLogic ausser das sie andere Namen haben ?

Warum gibt es keine Abstrakte RowLogic oder Gamelogic und diese bekommt das Spiel mit ?

Generell:
Ich halte von Wrapperklassen erstmal wenig, da sie dazu neigen falsche Abhaengigkeiten noch mehr zu verschleiern. Misko Hevery (ich glaub den hast du auch mal erwaehnt, ansonsten siehe Clean Code Talks bei youtube) meinte als Bsp einmal dass die Farbe der Tuerklinke nicht im Konstruktor eines Haus zu finden sein sollte (auch nicht als Wrapper), damit das Haus dann eine Tuerklinke erzeugen kann. Hier sind die Abhaengigkeiten falsch, das Haus sollte die fertige Klinke bekommen.
Kurz um: Zuviele Abhaengigkeiten zeugt immer von einer fehlenden Schicht der Abstraktion

 

[Antoras]

Ich kann meinen beiden Vorrednern nur zustimmen, das was du da machst ist vollkommen überabstrahiert. Ich würde das als klassenbasiertes und nicht als objektorientiertes Programmieren bezeichnen. Die Klassen, die mir spontan einfallen würden wären,

Board

,

Game

,

GameState

,

Position

und

Player

bzw.

Cell

.

Das Spielfeld kann durch ein 3x3 Array oder durch eine

Map<Position, Cell>

dargestellt werden, je nach dem was man bevorzugt und was besser auf die Situation passt. Das würde dann einfach zu lesenden und zu verstehenden Code ergeben, bei dem was du das hast muss man erst mal eine halbe Stunde mit dem Debugger durchsteppen um zu verstehen welche Methode welches Interfaces welchen Aufruf wohin delegiert.

Versuche mal das ganze Spiel nur durch immutable State zu realisieren, da wirst du sehr schnell feststellen, dass dein Ansatz nie zur Lösung führen kann. Immutability wird auch in oft empfohlenen Büchern wie Clean Code oder Effective Java angesprochen und ich kann ebenfalls nur empfehlen mal zu versuchen sich vorzustellen wie eine Lösung ohne mutable State aussehen könnte (was in deinem Beispiel eine mögliche Lösung darstellen würde).

Objektorientierung entsteht im Kopf und nicht im Code, dessen sollte man sich immer bewusst sein.

 

[Rudolf]

was mir am Design nicht sehr gefaellt ist dass es keine Abstraktionen gibt und sich Strukturen dem Anschein nach wiederholen.

Was ist der Unterschied zwischen FirstRowLogic, SecondRowLogic and ThirdRowLogic ausser das sie andere Namen haben ?

Wenn du genauer hingeschaut hättest, würdest du sehen, dass der Code komplett anders ist. Es werden andere Chainverbindungen aufgebaut.

Warum gibt es keine Abstrakte RowLogic oder Gamelogic und diese bekommt das Spiel mit ?

Ich wüsste nich wie man abstrahieren könnte.

Generell:
Ich halte von Wrapperklassen erstmal wenig, da sie dazu neigen falsche Abhaengigkeiten noch mehr zu verschleiern. Misko Hevery (ich glaub den hast du auch mal erwaehnt, ansonsten siehe Clean Code Talks bei youtube) meinte als Bsp einmal dass die Farbe der Tuerklinke nicht im Konstruktor eines Haus zu finden sein sollte (auch nicht als Wrapper), damit das Haus dann eine Tuerklinke erzeugen kann. Hier sind die Abhaengigkeiten falsch, das Haus sollte die fertige Klinke bekommen.
Kurz um: Zuviele Abhaengigkeiten zeugt immer von einer fehlenden Schicht der Abstraktion

Wenn du von Wrapperklassen wenig hältst, dann brechen deine Anwendungen ständig das Law of Demeter oder? Aber ok, ich wüsste zum Beispiel beim besten Willen nicht wie man eine zusätzliche Abstraktion einbauen könnte. Aber ich bin auch nicht der Gipfel der Programmiertechniken. Ich lasse mich gerne eines besseren Belehren. Wenn du so nett sein könntest, hier mal zu posten, wie du meinen Code refactorisieren würdest, würde ich deinen Code mit Interesse verfolgen.

Edit:
Ansonsten gebe ich euch allen Recht. Ich finde besonders die Klassen die mit GameLogic verbunden sind auch etwas merkwürdig (im Gefühl), aber von Arrays halte ich wenig und ich würde gerne soweit wie möglich darauf verzichten.

Edit2:
Und wenn wir schon einen konkreten Bezug zum Code haben. Würdet ihr meine Umsetzung vom State Pattern so lassen wie es ist, und würdet ihr enums dafür benutzen? Meine Umsetzung erfolgt durch normale Klassen die einen gemeinsamen Obertypen haben.

Edit3:
Wie viele Edits noch bis zu einer Antwort Ich habe meinen Code refactorisiert. Ich hatte anfangs dieses Bild: https://github.com/rudolfschmidt/TicTacToe/blob/master/Tic Tac Toe/src/model/logic/GameLogic.java
Das Konstruieren des Objekts sieht dann so aus:

		GameLogic logic = new GameLogic();
		logic.setFirstRowFirstFieldHandler(firstRowFirstFieldHandler);
		logic.setFirstRowSecondFieldHandler(firstRowSecondFieldHandler);
		logic.setFirstRowThirdFieldHandler(firstRowThirdFieldHandler);
		logic.setSecondRowFirstFieldHandler(secondRowFirstFieldHandler);
		logic.setSecondRowSecondFieldHandler(secondRowSecondFieldHandler);
		logic.setSecondRowThirdFieldHandler(secondRowThirdFieldHandler);
		logic.setThirdRowFirstFieldHandler(thirdRowFirstFieldHandler);
		logic.setThirdRowSecondFieldHandler(thirdRowSecondFieldHandler);
		logic.setThirdRowThirdFieldHandler(thirdRowThirdFieldHandler);
		logic.create();

Was meint ihr? Das ist das Bean Pattern. Und wenn ich das Initialisieren gleich im Konstruktor mache, damit meine State immutable ist, dann siehts echt unübersichtlich aus. Ich würde gerne mit euch über das Design der Anwendung sprechen. Es wurde ja bereits auf eine fehlende Abstraktionsschicht eingegangen, wenn zu viele Parameter initialisiert werden müssen. Kann dieser Punkt etwas ausführlicher beschrieben werden?

 

[nillehammer]

Nutze das Builder-Pattern. Damit hast du dann zwar genau so viele Aufrufe von Methoden wie bei den settern von Beans, aber der Client hat zu keiner Zeit eine Instanz, welche in einem ungültigen Zustand ist. Obwohl Du da keine final-Felder benutzen kannst, kann eine Instanz immutable sein, wenn es keine Modifikatoren gibt.

 

[Antoras]

[...]aber von Arrays halte ich wenig und ich würde gerne soweit wie möglich darauf verzichten.

Was hast du gegen Arrays? Oder gegen Maps? Sie sind einfach und man versteht sofort was gemacht wird. Ich habe deinen Code jetzt bestimmt 10 min angeguckt und verstehe noch immer nicht wie er funktioniert. Was ich meine was du versuchst hier umzusetzen ist die Logik des Spiels durch die Klassen zu modellieren. Aber warum? Java kennt sowas wie Methoden, die genau für solche Sachen wie die Überprüfung der Logik zuständig sind.

Zustandsautomaten sind ganz nett, sind bei manchen Problemen auch eine gute Lösung, aber hier sind sie mMn fehl am Platz. Jemand, der den Code liest, muss sich den Automat+Übergangsfunktion aufzeichnen um die Funktionsweise zu verstehen (also zumindest bei mir ist das so). Die Übergangsfunktion könnte man per Visitor-Pattern auch aus dem Automaten extrahieren (dann könnte man sie austauschen), das ist in Java aber so kompliziert umgesetzt, dass man am Schluss noch weniger verstehen würde. Außerdem haben die einzelnen Zustände kein weiteres Verhalten, weshalb ich ihren Einsatz für sinnlos erachte.

Apropos Zustand: Warum hat ein Spieler einen Zustand? Für mich hat ein Spiel einen Zustand, der Spieler ändert sich nie.

 

[KSG9|sebastian]

Builder und immutable:
Funktioniert doch...privater Konstruktor, den Builder als "inner class" und beim build() den Builder an den Konstruktor der "Model"-Klasse übergeben. Dadurch kannst du alle Instanzvariablen final machen...

 

[nillehammer]

Builder und immutable:
Funktioniert doch...privater Konstruktor, den Builder als "inner class" und beim build() den Builder an den Konstruktor der "Model"-Klasse übergeben. Dadurch kannst du alle Instanzvariablen final machen...

Dass immutable geht, habe ich nicht bestritten. Nur die Verwendung von final. Aber Du hast natürlich Recht, dass auch das geht. Mich persönlich hat aber die Duplizierung der zu merkenden Werte (einmal im builder und im gebuildeten selbst) immer genervt. Deswegen hab ichs nie so gemacht und diese Möglichkeit irgendwie aus meinem Kopf verdrängt opcorn:. Danke für die Klarstellung.

 

[bygones]

Wenn du genauer hingeschaut hättest, würdest du sehen, dass der Code komplett anders ist. Es werden andere Chainverbindungen aufgebaut.
[....]
Ich wüsste nich wie man abstrahieren könnte.

mhm du hast den Code aktualisiert, daher kann ich keine genauen Ratschlaege mehr geben. Im Grunde fande ich es komisch, dass nur Klassen und keine Interfaces etc gibt. Wie gesagt, ich weiss leider nicht mehr wie es war....

Wenn du von Wrapperklassen wenig hältst, dann brechen deine Anwendungen ständig das Law of Demeter oder

Ich denke wir reden im Grunde ueber das selbe. Wrapperklassen seh ich jedoch nur als reine Zusammenlagerung von Attributen, moeglicherweise die nicht in direktem Zusammenhang stehen. Es ist richtig, dass bei zu vielen Parametern schauen muss, ob a) die Abhaengigkeiten nur genutzt werden, um eine andere Abhaengigkeit zu initialisieren oder b) man nicht logische Zusammenhaenge gruppieren kann (also das wahr. was hier als Wrapperklassen besprochen wird).

Wenn du so nett sein könntest, hier mal zu posten, wie du meinen Code refactorisieren würdest, würde ich deinen Code mit Interesse verfolgen.

wie gesagt, ich finde den urspruenglichen code nicht mehr.

 

[Rudolf]

wie gesagt, ich finde den urspruenglichen code nicht mehr.

Paste2 - Viewing Paste 2432464

bittesehr

 

[KuhTee]

Das Problem ist, dass dein ganzer Code konzeptionell (nicht persönlich nehmen) Grütze ist. Wurde ja schon erwähnt. Und wenn das ganze Konzept nicht "rund" ist, dann wird der Code immer schlecht aussehen. Was vielleicht auch ein ganz guter Indikator ist: Wenn du ums verrecken komm raus keinen ansehnlichen Code hinbekommst, solltest du nochmal ganz an den Anfang.

Mehrere Hinweise und Vorschläge gab es hier ja schon.

 

[Antoras]

Wie gesagt, ich weiss leider nicht mehr wie es war....

Und ich hatte schon gedacht mit einem Versionskontrollsystem könnte man den Krankheitsverlauf schizophreniegestörter Javaentwickler nachvollziehen.

 

[Rudolf]

KuhTee, dann sag doch mal konkret, was schlecht sein soll.

Die hier genannten Hinweise machen den Code meiner Meinung nach nicht besser. Zum Beispiel fängts mit arrays an, geht dann über maps und landet letztendlich bei einem "dein Code ist weg". Außerdem ist das so eine stelle, das ich immer wieder habe.

Also nicht nur kritisieren, sondern mal konkret sagen, was schlecht ist und wie man es verbessert statt "mach mal von Vorne alles".

 

[KuhTee]

KuhTee, dann sag doch mal konkret, was schlecht sein soll.

Schlecht ist zum einen, dass jedes Field seinen eigenen FieldHandler hat, welcher auch noch hardcoded ist. Dadurch wird dein Code endlos komplizierter und gewinnt NULL dabei. Mach doch mal aus deinem 3x3 Tic-Tac-Toe ein 5x5 Tic-Tac-Toe oder ein Tic-tac-Toe für N Spieler. Mit einem guten Softwaredesign sollte das ein Klacks sein.

Du versuchst das Ermitteln einer Reihe über unnötig umständliche Bedingungen zu realisieren statt es einfach mathematisch anzugehen.

Du hast das mit dem OOP deutlich übertrieben. Du musst dich immer fragen "was gewinn ich aus einer bestimmten Projektstruktur?". In deinem Fall seh ich da nichts, du verlierst nur Flexibilität. "Verbessern" würde ich da nichts, ich würds komplett neu schreiben. Nimm mir das nicht übel. Ich mein, du hast durchaus sauber programmiert. Aber das Softwaredesign ist leider nicht sauber. Ich versteh durchaus dein Ziel, du möchtest ein sauberes Projekt nach allen Regeln der OOP entwickeln. Nur die Eleganz hast du dabei vergessen.

Zum Beispiel fängts mit arrays an, geht dann über maps[...]

Schau dir doch mal ein Tic-Tac-Toe Spielfeld an. Woran erinnert das einen sofort?

Ich würde den Code als missglückten Versuch abhaken, und es noch einmal nach dem KISS Prinzip versuchen. Kannst dabei nur lernen, und das ist doch das wichtigste.

 

[Antoras]

Also nicht nur kritisieren, sondern mal konkret sagen, was schlecht ist und wie man es verbessert statt "mach mal von Vorne alles".

Nö, wir geben Review/Feedback. Du bist derjenige der sie annehmen und darüber nachdenken muss. Es wurden schon einige Punkte genannt, die du aber ohne weitere Begründung abgelehnt hast.

Niemand sagt etwas wenn man mal ein kleines Beispiel machen soll, aber wir haben auch noch etwas anderes zu tun und deswegen wollen vermutlich nur die wenigsten komplette Lösungsvorschläge anbieten, deren Implementierung gleich mal eine halbe Stunde und länger in Anspruch nehmen kann. Selbst wenn man alles auf das nötigste reduziert ist die Zeit einfach zu schnell vorbei.

 

[JohannisderKaeufer]

Mal ein konkreter, wenn noch nicht ganz fertiger und bis ins letzte Detail ausgearbeiteter Vorschlag.
View und Controller bleiben also aussen vor.

package model;

import java.util.Scanner;

public class Game {

    private GameState gameState;

    public Game(GameState gameState) {
        this.gameState = gameState;
    }

    public void mark(int position) {
        try {
            gameState = gameState.mark(position);
        } catch (Exception e) {
            System.out.printf("####################%n%s%n####################%n", e.getMessage());
        }

    }

    public String toString() {
        return gameState.toString();
    }

    public static void main(String[] args) {
        Game game = new Game(GameState.INITIAL_GAME_STATE);

        Scanner scanner = new Scanner(System.in);

        while (true) {
            System.out.println(game);
            System.out.println("Please select a field");
            if (scanner.hasNext()) {
                game.mark(scanner.nextInt());
            }
        }
    }
}

Das Spiel beinhaltet eigentlich nur einen GameState und die Methode zum setzen auf ein Feld.

package model;

import model.board.Board;
import model.player.Player;

public class GameState {

    public static final GameState INITIAL_GAME_STATE = new GameState(Player.A, Player.B, Player.A, Board.EMPTY);

    private final Player a;
    private final Player b;
    private final Player onTurn;
    private final Board board;

    public GameState(Player a, Player b, Player onTurn, Board board) {
        this.a = a;
        this.b = b;
        this.onTurn = onTurn;
        this.board = board;
    }


    public Player getWinner() {
        return null;
    }

    public GameState mark(int position) {
        Board newBoard = board.mark(onTurn.getMarking(), position);
        return new GameState(a, b, onTurn == a ? b : a, newBoard);
    }

    public String toString() {
        return String.format("On turn: %s%n%s ", onTurn, board);
    }
}

Das einzig interessante ist, das die mark Methode einen neuen GameState zurückliefert.

package model.board;

public class Board {

    public static final Board EMPTY = createEmtpyBaord();

    private final Field[] fields;

    public Board(Field[] fields) {
        this.fields = fields;
    }

    public Board mark(Marking marking, int position) {
        Field[] newfield = fields.clone();
        newfield[position] = newfield[position].changeTo(marking);
        return new Board(newfield);
    }

    public String toString() {
        return String.format("%s %s %s%n%s %s %s%n%s %s %s", fields);
    }

    private static Board createEmtpyBaord() {
        Field[] fields = new Field[9];
        for (int i = 0; i < fields.length; i++) {
            fields[i] = new FreeField();
        }
        return new Board(fields);
    }
}

package model.board;

public interface Field {
    Marking getMarking();

    Field changeTo(Marking marking);
}

package model.board;

public class FreeField implements Field {
    @Override
    public Marking getMarking() {
        return Marking.FREE;
    }

    public Field changeTo(Marking marking) {
        return new MarkedField(marking);
    }

    public String toString() {
        return Marking.FREE.toString();
    }
}


package model.board;

public class MarkedField implements Field {
    private final Marking marking;

    public MarkedField(Marking marking) {
        this.marking = marking;
    }

    public Marking getMarking() {
        return marking;
    }

    public Field changeTo(Marking marking) {
        throw new IllegalStateException("Field is already marked");
    }

    public String toString() {
        return marking.toString();
    }
}

package model.board;

public enum Marking {

    A("X"), B("O"), FREE(".");

    String sign;

    private Marking(String sign) {
        this.sign = sign;
    }

    public String toString() {
        return sign;
    }
}

Sowie schlußendlich Player

package model.player;

import model.board.Marking;

public enum Player {
    A(Marking.A), B(Marking.B);

    private final Marking marking;

    private Player(Marking marking) {
        this.marking = marking;
    }

    public Marking getMarking() {
        return marking;
    }
}

Was noch fehlt, ist das Überprüfen ob das Spiel beendet ist und wer gewonnen hat. Dies sollte aber keine allzugroßen Schwierigkeiten bereiten.

Das ganze ist ohne überdimensionierte Konstruktoren und unzählige Klassen bewerkstelligt. Veränderbarer State ist weitgehendst vermieden. Auch ohne Builder oder Beans Pattern.
Klar könnte man das ganze noch einfacher halten, aber dann hätte es wohl nur noch sehr wenig mit OO zu tun.