Krautkanal.com

Veröffentlicht am 2016-11-25 21:53:21 in /prog/

/prog/ 9544: CleanCode

souuf Avatar
souuf:#9544

Bernd will Variante A umsetzen. Ist das mehr oder weniger CleanCode? Bernd will Busniesslogiken von ModelKlassen trennen und glaubt es ist gut seine Businesslogiken daher in Hilfsklassen auszulagern die wiederum auf das Model also die Daten zugreifen.

Will man den Preis des Eis in Rubel haben muss man die Hilfsklasse nutzen. Ist das gut so?

slaterjohn Avatar
slaterjohn:#9545

Da eine Klasse nur eine Sache tun soll, legt Bernd keine Methode Preis(währung) an der er die währung übergibt um den Preis in dieser Währung zu erhalten sonden legt für jede Währung eine Methode an.

Also ist Variante A die beste Lösung?

greenbes Avatar
greenbes:#9546

Wie wär's mit einer Währungsrechnerklasse?:

Waehrungsrechner.convertEuroToMark(Vanilleeis.getPreis)

amboy00 Avatar
amboy00:#9547

>>9546
Ja, das kommt auf meine Lösung raus nur mit anderen Klassennamen.

Hilfsklasse.convertEuroToMark(Eis.getPreis)

Kann ich also davon ausgehen das die Lösung mehr oder wenig "sauber" ist?

Gibts es noch ganz andere Lösungen? Schön wäre, wenn der Ausdruck zum ermitteln der Preise ganz kurz ist und nicht so lang wie
Waehrungsrechner.convertEuroToMark(Vanilleeis.getPreis)
Das sind fast 60 Zeichen.

albertaugustin Avatar
albertaugustin:#9548

Noch eine viel bessere Idee:

Eis.getPreisInRubel(){
Hilfsklasse.getPreisInRubel(getPreis());
}

Genial oder?

-Es ist ein super Kurzer Ausruck
statt:
Hilfsklasse.getPreisInRubel(Eis.getPreis)
ist: kurzer Ausruck:
Eis.getPreisInRubel()

- Businesslogik ist in einer Hilfsklasse, nur der Aufuruf der Businesslog ist in der Model Klasse

Das müsste doch eigentlich eine gute lösung sein.

yigitpinarbasi Avatar
yigitpinarbasi:#9550

[sage]
Warum nicht als Argument?
getPrice(RUBEL);
[/sage]

trickyolddog Avatar
trickyolddog:#9551

>>9546
Dies und nur dies.

>>9547
Nein es kommt nicht auf das gleiche raus. Deine "Hilfsklasse" (denk dir mal einen echten Namen für die Klasse aus, dann wird dir auffallen, dass das Stuss ist) braucht eine Referenz auf das Eis. Der Währungsumrechner kriegt einfach nur eine Zahl und gibt eine andere Zahl aus.

rdbannon Avatar
rdbannon:#9552

>>9550
Dadurch hätte dein Model "Eis" Businesslogik das sie eigentlich nciht haben soll weil ein Model nur Daten beinhaltet.

Eine Methode soll nur eine Sache können. Wenn die Preis(Währung x) Methode anhand der Währung einen Preis zurückt gibt wird der INhalt von Preis(Währung x) ziemlich fett. Da würde man eher einen eigenen WechslekursService-Klasse anlegen.

haydn_woods Avatar
haydn_woods:#9553

An sich hat OP recht mit Variante A, allerdings sollte man anmerken, dass OOP Krebs ist (drinbevor hurrdurr aber jeder macht OOP) und manchmal weniger restriktive Implementierungen vorzuziehen sind.

drinbevor "a-a-aber meine best practices!!!":
Niemand kehrt. Entwickelt ruhig TestFirst wenn ihr wollt, sinnvoll ist das aber weniger.

juaumlol Avatar
juaumlol:#9555

Wieso darf eine Modellklasse keine Geschäftslogik enthalten? Wenn OOP Krebs, dann richtig. Logik und Daten gehören doch zusammen. Also ich würde getPreis(Währung w) { Währungsrechner.convertiere(Währung.EUR, w, this.getPreis()) } machen

xspirits Avatar
xspirits:#9556

>>9555
Bist du behindert?

Wie Teste ich denn den behindertsten Getter überhaupt? Wie soll ich den Währungsconverter da drin mocken? Du bist schon ein bisschen Mongo.

eloisem Avatar
eloisem:#9557

>>9556
>Wie soll ich den Währungsconverter da drin mocken?

Anderer Bernd hier. Mit Mocken meinst du einen Junit-Test?
Du würdest nach belieben die Eis-Klasse Oder Währungsrechner.convertiere() aufrufen und Daten rein schmeißen und den Return wert testen.

Stell dich nicht so dumm an. Erkläre was dein Problem ist.

jimmywebdev Avatar
jimmywebdev:#9559

>>9557
>>9556
JUnit != Mocking
Sowas wäre ein einfacher Assert-Test und bräuchte nicht einmal Mocking, weil es keine Cross-Class-Interactions gibt.

>>9555
> Wenn OOP Krebs, dann richtig. Logik und Daten gehören doch zusammen.
Nicht immer Penis

suprb Avatar
suprb:#9560

>>9544
Eine Klasse kapselt Daten und bietet Methoden um damit zu interagieren. Dein Beispiel A hat völlig unnötig zwei Klassen, wovon eine nur Methoden enthält.

Willst du das trennen, dann würde es reichen die drei Methoden als Funktionen ohne Klasse umzusetzen.

Stattdessen kannst du aber auch eine Klasse "Verkaufsstand" machen, die einen Array "Produkte" hat, wo Eis drin ist. Eis bietet dann eine methode "PreisinCent" und Verkaufsstand hat eine Methode "Preise(Währung)" und eine "verkaufe(Produkt, Währung){assert Produkt in Produkte;return convertPreis(Währung, Produkt.getPreis);}"

kuldarkalvik Avatar
kuldarkalvik:#9563

>>9559
Gib Kot wie ich einen Getter teste der je nach Zeit einen Preis in eine Währung umrechnet. Könnte ja recht komplex dahinter sein...

subburam Avatar
subburam:#9564

Nach dem KISS Prinzip:
Brauchst du den Logik Code irgendwo anders auch? -> Hilfsklasse/-modul.
Ist er nur für diese eine Klasse relevant? -> Mach es Teil der Klasse.
Ist es für eine Gruppe von Klassen relevant? -> Vererbung o.ä.

Wenn sich das später mal ändert kannst du es immer noch mit wenig Aufwand ändern.

illyzoren Avatar
illyzoren:#9569

>>9564
Gute Antwort.

Möchte noch eine weitere Perspektive einbringen. Meiner Meinung/Erfahrung nach sind statische Methoden in Java ziemlicher Krebs. Ja, OOP ist auch Krebs, alles ist Krebs, schon klar, aber in Java ist es halt Mist mit den statischen Methoden, da sie nie Teil von Interfaces sein können und solche Dinge, man handelt sich damit oft Probleme ein. Ich verwende sie möglichst nur noch für private Funktionalität in einer Klasse.

Das wäre mein Alternativvorschlag:

class Eis {

  // something something

  private final double roherPreis;
  private final FlexiblerPreis flexiblerPreis;

  public double getRoherPreis() {
    return roherPreis;
  }

  public FlexiblerPreis getFlexiblerPreis() {
    return flexiblerPreis;
  }

  // etc.
}

class FlexiblerPreis {
  private finale double roherPreis;

  FlexiblerPreis(double roherPreis) {
    this.roherPreis = roherPreis;
  }

  public double getPreisInRubel() {
    return roherPreis * 100;
  }

  // etc.
}


Die Verwendung als "eis.getFlexiblerPreis().getPreisInRubel()" ist sogar etwas weniger umständlich als mit der Hilfsklasse.

ovall Avatar
ovall:#9570

>>9552
class Eis {

// something

static getPrice(Währung){
// something
}}
?

yesmeck Avatar
yesmeck:#9578

>>9570
Nein, Bernd.
Kleine Erklärung: Du hast mehrere Objekte Eis. Jedes dieser Objekte hat einen eigenen Preis. Das heißt, jedes Objekt braucht seine eigene Methode um seinen Preis zurückzugeben. Der Währungskoverter ist in eine Hilfsklasse ausgelagert, da der Kot von Klassen geteilt werden kann und keine Instanzvariablen verändert, sondern nur eine Methode an einem übergebenen Parameter aufruft (daher keine eigene Objektinstanz braucht) und einen Wert zurückgibt. So einfach.

Anderes Beispiel:
Du baust Autos. Du legst einfach EINEN Preis für ALLE Autos fest. Dieser Preis wäre dann static, weil alle Objekte der Klasse Auto ihn sich teilen, und kein Auto einen eigenen, einzigartigen Preis hat (Klassenvariable). Dann deklarierst du auch den getter für den Preis static.
Außerdem willst du noch den Kilometerstand festhalten. Der ist natürlich für jedes Auto unterschiedlich, jedes Auto hat seinen eigenen. Deshalb ist der KM-Stand nicht static (Instanzvariable), auch nicht seine setter und getter (du willst ja auf eine Instanzvariable zugreifen). Wenn du jetzt Logik entwickeln willst, die dir den Stand in Meilen gibt, würdest du eine Hilfsklasse Konverter erstellen, da es keinen Sinn macht im Autoobjekt selbst sowohl Meilen als auch KM festzuhalten. Die Hilfsklasse und ihre Methoden sind statisch, da du keine Konverterinstanz brauchst, und sie nur eine statische Variable für die Umrechnung enthält (ist ja immer das Selbe). Das Ganze kommt in eine eigene Klasse, es könnte ja sein dass du auch noch LKWs bauen willst, und die auf die selbe Logik zugreifen wollen.

Die Methoden der Hilfsklasse sind statisch, weil die Hilfsklasse selbst keine Daten enthält (bzw. nur static), und du keine Instanzen von ihr erstellst, sondern nur Funktions Methodenaufrufe tätigst.

Kleine Übersicht, wenn statische Methoden zu nutzen sind, falls du das obere Beispiel ferstanden hast:
1. Wenn die Methode keine Instanzvariable nutzt
2. Wenn der Funktionsaufruf nicht von der Erstellung neuer Objekte abhängt (außer du willst static initializers nutzen, aber das ist wieder ein anderes Thema)
3. Wenn du Hilfsklassen schreibst, die immer konstanten Inhalt haben (Bsp: mathematische Formeln)
4. Wenn du sicherstellen willst, dass Methoden nicht verändert/überschrieben werden
5. Kot, der von Instanzen geteilt wird, sollte in statische Methoden ausgelagert werden (die ganze Idee hinter Hilfsklassen)

OOP ist behindert

LG BRENT

herrhaase Avatar
herrhaase:#9581

>>9569
Dein Vorschlag ist ein billiger Abklatch von >>9548

Dein Vorschlag hat Nachteile:
-Datenobjekt Eis ruft Businesslogik (FlexiblerPreis) auf, somit sind BusinessLogiken immernoch im Datenobjekt

shoaib253 Avatar
shoaib253:#9620

>>9544
Das Beispiel ist vielleicht ein bisschen daneben.

Du verwendest Preis in Eis, und jeder Preis hat auch immer eine Währung (du kannst keinen "währungsneutralen" Preis angeben). Die Währung dann plötzlich zu versuchen aus Eis rauszuziehen ergibt keinen Sinn, dann dürfte dein Eis die Eigenschaft Preis gar nicht haben.
Dementsprechend erscheint mir das hier >>9555 die sinnvollste Lösung, auch weil diese Aussage korrekt ist:
>Logik und Daten gehören doch zusammen.

Verstehe auch nicht das Problem, warum getPreis schwierig zu testen sein sollte.

Im Übrigen, insbesondere wenn das hier Java sein soll, ist der Datentyp double für Geld eine denkbar schlechte Wahl, dafür nimmt man BigDecimal.

funwatercat Avatar
funwatercat:#9621

>>9620
Wie wäre es mit unsigned int für den Preis in Cent?

bassamology Avatar
bassamology:#9622

>>9621
Warum nicht einfach in Keksen rechnen?

kennyadr Avatar
kennyadr:#9625

>>9621
>java
>unsigned int

Neuste Fäden in diesem Brett: