C# Parameter (CleanCode Tipp)

[Ghost_Rider_R]

Hallo zusammen,

ich bräuchte mal einen Tipp, der in die Richtung CleanCode / Best Practice / Guter Stil gehört.

Ein blödes Beispiel:

Wir haben eine Klasse Fahrzeug:
-Anzahl Achsen
-Anzahl Türen
-Anhänger Kupplung
-Tankvolumen
-Fahrzeuggewicht
-Leistung
-Höchstgeschwindigkeit
-CW-Wert
-usw.

Jetzt habe ich eine Methode, welche den Benzinverbrauch schätzt. Welche Variante wäre hier zu bevorzugen und warum?

public double schaetzenVerbrauch(Fahrzeug fahrzeug){...}
oder
public double schaetzenVerbrauch(Fahrzeuggewicht, CW-Wert){...}

Die erste hat für mich den Vorteil, dass ich in jede andere Methode ,,einfach" immer nur das Fahrzeug packen kann und die Methode macht was damit, ohne dass ich das Objekt zerpflücken muss z.B.:

public decimal schaetzenFahrzeugWert(Fahrzeug fahrzeug){...}
Alternativ wäre
public decimal schaetzenFahrzeugWert(Anzahl Achsen, Anzahl Türen, Fahrzeuggewicht, Leistung,...){...}

Beim der zweiten Variante, hat die Methode aber immer nur soviel Informationen wie unbedingt notwendig sind.

Was wäre in solch einem Fall die zu bevorzugende Lösung und warum?

Ich bin gespannt...

LG Ghost Rider

 

[Maviapril2]

Aendert sich die Benamung innerhalb der Fahrzeug-Klasse, muss bei Variante "Fahrzeug fahrzeug" jede Funktion, in der die jeweilige Variable verwendet wird, angepasst werden, bei der anderen Variante nicht. Wird jetzt in dem Fall wahrscheinlich nicht passieren, kann aber in anderen Faellen durchaus vorkommen...
Andererseits ist es damit besser lesbar und einfacher zu coden

 

[cmi777]

Ich würde die zweite Variante bevorzugen, da diese Methode sehr explizit und transparent ist. Positiver Nebeneffekt: Dadurch lässt sie sich auch sehr einfach testen (keine Mocks benötigt).

Es hängt glaub ich immer auch ein bisschen damit zusammen, wieviele Parameter es sind und wie stark die Methode bzw. die Klasse mit der "Fahrzeug"-Klasse verbunden sind. Mitunter macht es auch mehr Sinn, direkt das Objekt reinzugeben.

Ich fahre bei meinen Projekten immer die Linie, möglichst datensparsam zu sein. Dinge wie "bei Änderung der Benamsung muss alles geändert werden" lasse ich außen vor, das übernimmt die IDE und ist mir egal.

 

[Ghost_Rider_R]

Ja, das sind auch meine Ansichten, jedoch denke ich, dass es hier bestimmt so etwas wie eine ,,empfohlene" Vorgehensweise gibt, oder täusche ich mich und beides ist in der Praxis gängig?

 

[DerZock]

Das sauberste wäre wohl die Verwendung eines Interface:
public decimal schaetzenFahrzeugWert(IFahrzeug fahrzeug){...}

 

[Ghost_Rider_R]

Wg Interface: Naja, ich würde die Sache nun nicht noch mehr aufblasen als unbedingt notwendig. KISS und so 😄

 

[Yuuri]

"Musst" du das so machen? Bau dir eine Klasse BenzinverbrauchCalculator und übergib ihr das Fahrzeug.

jedoch denke ich, dass es hier bestimmt so etwas wie eine ,,empfohlene" Vorgehensweise gibt, oder täusche ich mich und beides ist in der Praxis gängig?

In der Praxis wird alles gemacht.

KISS und so 😄

Genau deshalb nimmt man Interfaces, damit man keine strikte Bindung hat... :O

 

[Ghost_Rider_R]

"Musst" du das so machen?

Nein, das ist nur für mich, um meinen eigenen Code zu optimieren. Über die Frage bin ich letztens gestolpert und konnte mich nicht so recht festlegen, was hier die bessere Variante ist.

Denkbar wären natürlich auch sensiblere Anwendungsfälle wie z.B.:

Person:
Vorname
Nachname
Alter
Geschlecht
!!! Gehalt !!!
!!! Religion !!!

Ergänzung (28. September 2021)

Dann bekäme man mit z.B. schätzenWohnort(Person person) {...} ja viel mehr Informationen übergeben, als eigentlich notwendig wären und man weiß nie wirklich, welche Informationen bei der Person angegegeben sein müssen, damit die Funktion ordnungsgemäß arbeitet.

Da wäre schätzenWohnort(Vorname, Alter, Geschlecht, Gehalt) transparenter, aber eben auch unübersichtlicher und aufwendiger.

Das war auch so eine Überlegung...

 

[tollertyp]

Wieso nicht beide Varianten? Und die erste delegiert an die zweite.

Die Benamung finde ich aber schlecht. Ich würde nicht "schaetzenVerbrauch" sondern "schaetzeVerbrauch" machen. Und in C# wohl eher "SchaetzeVerbrauch".

Code sollte sich lesen lassen, und schätze Verbrauch liest sich besser als schätzen Verbrauch.

 

[Hancock]

Richtige Vorgehensweise/Best Practice:
1.) Englisch
2.) Ne Klasse und Klassenmethoden
3.) Inheritance, falls notwendig/"is a"-Beziehung

public class Vehicle{
public  virtual double consumptionEstimate(){
return a*x;
}
}
public class Hatchback:Vehicle{
public override double consumptionEstimate()
{
return a*y;
}
}

 

[SomeDifferent]

Für reine mathematische Berechnungen übergebe ich immer ausschließlich die Parameter, die auch hierfür notwendig sind. Alles andere erzeugt nur Overhead und unnötige Abhängigkeiten. Wie bereits genannt wird das Testen schwieriger und die Wiederverwendbarkeit leidet.

@Hancock

Dann aber bitte über eine Schnittstelle IFuelConsumer o.ä.

 

[tollertyp]

consumptionEstimate <-- sorry, aber bei der Benamung stehen mir die fehlenden Nackenhaare zu Berge.
Entweder estimateConsumption oder consumtionEstimation.
Das Verb am Ende ist ziemlich igittigitt.

Unabhängig davon würde ich wohl auch die Methode ins Fahrzeug legen, die eigentliche Funktion zur Berechnung kann auch wo anders liegen (via Delegation). Andererseits wenn die Klassen konsequent nur dumme POJOs sind, dann würde ich mir gut überlegen, ob ich hier den Mischmasch beginne.

Wenn man beide Varianten hat, ist das Testen auch einfach. Man hat eine Methode und eine zusätzliche Convenience-Methode.

 

[cmi777]

Willkommen in der wunderbaren Welt der IT wo es auf alles genau eine Antwort gibt: "It depends"

Wenn du ein Objekt reingibst, würde ich auch ein Interface in Erwägung ziehen.

Ich würde glaube ich die Grenze ziehen was die Methode macht. Da es sich um eine Methode handelt (und keinen Konstruktor) würde ich glaub ich zu Parametern greifen. Anderenfalls würde man das Objekt wahrscheinlich schon via DI/Konstruktorparameter in die Klasse geben und dann kann man mit der Klassenproperty arbeiten.

Also Beispielcode/Pseudocode (ist sehr verboses PHP aber ist ja wurscht):

class FahrzeugService {
    private FahrzeugInterface $fahrzeug;

    public function __construct(FahrzeugInterface $fahrzeug) {
        $this->fahrzeug = $fahrzeug;
    }

    public function calc(): float {
       // do stuff with $this->fahrzeug
    }
}

Im konkreten Fall ist es aber scheinbar anders: du hast 2 Werte und willst aus diesen Werten etwas errechnen. (also quasi eine Art add oder multiply - da würde ja keiner auf die Idee kommen ein Objekt reinzugeben) Also gib die Werte rein. Wenn deine Klasse/deine Methode etwas mit oder auf Basis eines Objekts berechnen soll, dann gib das Objekt rein.

Ist halt wirklich ein "it depends" und kann mMn nicht generell beantwortet werden. Es kommt extrem auf den konkreten Usecase an.

 

[tollertyp]

Und es gibt bei Clean Code auch nicht die eine Wahrheit. Wichtig ist zu wissen, warum man sich für eine Variante entschieden hat.

Arbeitet man an bestehendem Code, dann bevorzuge ich es mitunter sogar manche Anti-Patterns durchzuziehen (z.B. wie deutsche Sprache im Code) und lieber konsequent zu bleiben, als einen Mischmasch zu beginnen. Wenn man sich entscheidet, das aufzuräumen, dann sollte man es auch zugüg und konsequent machen.

 

[Hancock]

Das Verb am Ende ist ziemlich igittigitt

Estimate ist auch ein Substantiv, aber ja, wie rum und wie man die Methode benennt ist ja dann ne Stilfrage.

Dann aber bitte über eine Schnittstelle IFuelConsumer o.ä.

Das kommt auf das Projekt an. Wenn es nur Fahrzeuge geben wird, dann ist ein IFuelConsumer sehr Enterprisey, wenn es auch mal andere Spritfresser geben kann, dann ja, auf jeden Fall.

 

[Ghost_Rider_R]

Und es gibt bei Clean Code auch nicht die eine Wahrheit. Wichtig ist zu wissen, warum man sich für eine Variante entschieden hat.

Vermutlich ist es wirklich so wie du sagst, es ist beides möglich und alles hat seine Vor- und Nachteile. Das mit dem Anti-Pattern / Deutsche Sprache mache ich tatsächlich auch immer so, einfach weil ich es damals so gelernt habe und für mich am besten lesbar ist und ich es auch so gewohnt bin.

 

[Hancock]

Allerdings fragst du nach Best-Practice und das ist es auf jeden Fall nicht.

 

[Yuuri]

Dann bekäme man mit z.B. schätzenWohnort(Person person) {...} ja viel mehr Informationen übergeben, als eigentlich notwendig wären und man weiß nie wirklich, welche Informationen bei der Person angegegeben sein müssen, damit die Funktion ordnungsgemäß arbeitet.

Dann erstell ein Interface, welches nur benötigte Werte enthält und implementiere das in deinem großen Objekt. Interfaces sind genau dafür geschaffen worden.

Du nutzt OOP, also hör auf Werte einzeln zu übergeben. God Objects brauch auch keiner. Interfaces lassen sich eben auf das runterbrechen, was benötigt wird und Klassen können diese implementieren.

 

[BeBur]

Gibt es optionale Parameter? -> Method Chaining
Werden gleiche Parametergruppen öfters verwendet? -> Parameter Object (ggf. Interface bei C#?)
Evtl. macht die Funktion mehrere Dinge auf einmal -> Aufteilen
Generell +1 nur die Parameter, nicht das ganze Objekt, aus den genannten Gründen.

2.) Ne Klasse und Klassenmethoden

C# beherrsche ich nicht, ich ging jetzt einfach mal davon aus, dass die benannten Funktionen public Methoden der Klasse schon bereits sind?

 

[Tornhoof]

Der klassiker gemäß den üblichen OOP Büchern wäre wahrscheinlich sowas:
Klasse für die Berechnung, irgendeine Factory Methode/klasse bis zu Abstract Factory Pattern um die eigentliche Berechnungsklasse zu erzeugen, die Berechnungsklasse bekommt dann dein IFahrzeug übergeben.
(gerade für unsere Java/Design Pattern Fanatiker)

In der Realität, stellt sich die Frage: Was ist dein Businessproblem?

Wenn dein Businessproblem die Berechnung von Verbrauch ist und du mehr als nur ein Auto unterstützen musst, macht eine solche Abstraktion Sinn, in der Realität wird sie ja natürlich weit komplizierter sein, die Verbrauchsberechnung von Fahrzeugen ist kein triviales Thema.

Wenn dein Businessproblem ein Anderes ist und der Verbrauch nur ein Seiteneffekt ist, dann ist es sowieso egal und sollte wahrscheinlich gestrichen werden

Oder ums einfach zu sagen, versuche nicht an Hand von abstrakten, ach so einfachen Beispielen, Rückschlüsse auf Best Practice zu ziehen, sie werden zu 99% falsch sein.