Sauberes Design, Clean Code, etc. pp.

[maki]

Hab gestern einen IMHO sehr interessanten Artikel gelesen: When to Break Apart Your Application | Javalobby

Was mich am meisten beeindruckt hatte:

Matt told us about his experience teaching kids in Africa to program. While teaching programming, Matt observed that giving the children the single rule that they shouldn't have methods longer than 10 lines resulted in the children writing code with SRP, encapsulation, and other positive attributes. Matt argued that beginning with a simple rule that could get you 80% of what you want is likely better than 10 rules that, if followed correctly, could get you 100% of what you want.

Wobei 10 Zeilen pro Methode mir pesönlich noch zu lang sind

Trotzdem eine sehr interessante Einsicht:
Mit einer einzigen Regel, kann man schon recht guten Code schreiben.

Mir ist eine Regel natürlich zu wenig und deswegen mache ich 2 draus:
1. Methoden haben kurz zu sein.
2. Methoden haben noch kürzer zu sein!
(frei nach Bob Martin)

Habe mich letztens mit einem alten Ada Profi unterhalten, so unterschiedlich wir Dinge auch nennen, er sagte doch tatsächlich: "Ein sauberes Programm erkennt man daran, dass es viele kleine Quellcodedateien gibt.", dem musste ich natürlich zustimmen.

Wie seht ihr das?
Könnte es wirklich so einfach sein?

 

[Andgalf]

Na ja so einfach würde ich jetzt nicht sagen ... aber wenn man sich an ein bis zwei Grundregeln hält steigt die Code-Qualität schon ziemlich erheblich.

Ich würde eine dritte Regel beifügen ...

- Methoden sollten so wenig Parameter bekommen wie möglich, jedoch in keinem Fall mehr als drei.

Allerdings hält man diese Regel bei kurzen Methoden wahrscheinlich schon automatisch ein.

[EDIT]

Matt argued that beginning with a simple rule that could get you 80% of what you want is likely better than 10 rules that, if followed correctly, could get you 100% of what you want.

Da ist auf jeden Fall was wahres dran
[/EDIT]

 

[maki]

Zusätzliche Regeln würden mir auch einfallen, das Problem ist, wenn sich mal ein paar gute Regeln etabliert haben, wird das ganze doch recht komplex, nicht nur für Anfänger, auch für Leute die zB. seit Jahren/Jahrzehnten einen bestimmen "Stil" haben.

Allerdings hält man diese Regel bei kurzen Methoden wahrscheinlich schon automatisch ein.

Ich denke das ist nicht nur für diesen Fall wahr, in kurzen Methoden kann ich einige der klassischen Design/Struktur Fehler aus pragmatischen Gründen nicht mehr so einfach machen und werde vielleciht nicht gezwungen aber zumindest "gedrängt" Prinzipien wie SoP, SRP, keine Redundanzen, etc. pp. einzuhalten.. selbst wenn man diese Prinzipien gar nicht (im Detail) kennt?

Wenn eine (oder 2 ) Regeln wirklich schon einen Großteil der üblichen Probleme erschlägt und gleichzeitig einfach zu merken ist, wäre das imho schon sehr viel Wert.

Wenn ich (Legacy) Code refactoren muss, mache ich das meistens als aller erstes: Extract Method
Kostet viel Zeit, wenn ich mir alleine das sparen könnte...

 

[schalentier]

Allein mit dieser Regel koennte man eine 1000LOC Methode einfach in 100 10LOC Methoden trennen und sie durchnummerieren. Also imho braucht man schon noch ein paar weitere Regeln:

- Sprechende Namen (Methoden, Variablen und Klassen)
- DRY
- Einfachheit (verstaendliche Algorithmen, flache Vererbungshierarchien, Standardlibrary benutzen)
- keine Probleme versuchen zu loesen, die man noch nicht hat

Fuer mich die wichtigste aller Regeln ist, alles so einfach (sogar moeglichst trivial) zu halten wie nur irgendwie moeglich. Daraus kann ich fuer mich das meiste was im Clean Code steht irgendwie ableiten ;-)

 

[maki]

Allein mit dieser Regel koennte man eine 1000LOC Methode einfach in 100 10LOC Methoden trennen und sie durchnummerieren.

Der Punkt ist aber, dass es sich hier nicht "direkt" um Refactoring handelt, sondern darum, wie der Code von Anfang auszusehen hat
Dadurch schreibe ich nciht zuerst schrottigen 100 Zeilen pro Methode Code, sondern muss von Anfang an bessere Lösungen finden.

- Sprechende Namen (Methoden, Variablen und Klassen)

Das hängt stark mit der Methodengröße zusammen imho, je kleiner die Methoden umso mehr Methoden und damit auch Platz für Namen, die immer weniger auszudrücken haben -> einfacher mit kleinen Methoden

- DRY

Redundazen entstehen oft (aber nicht nur) wenn man zB. Code per C&P einfügt, anstatt ihn in eine eigene Methoden zu extrahieren. Für die anderen Fälle sind dann natürlich andere Regeln einzuhalten... zB. Kapselung & Information Hiding

- Einfachheit (verstaendliche Algorithmen, flache Vererbungshierarchien, Standardlibrary benutzen)

Kleiner ist imho einfacher
Auch hier baucht man mehr regeln...

- keine Probleme versuchen zu loesen, die man noch nicht hat

Das ist einer der am häufigsten Fehler ime, hat seine Ursachen aber nicht auf Code Ebene imo.

Fuer mich die wichtigste aller Regeln ist, alles so einfach (sogar moeglichst trivial) zu halten wie nur irgendwie moeglich. Daraus kann ich fuer mich das meiste was im Clean Code steht irgendwie ableiten

Ich sehe da wirklich eher die Regel "kurze Methoden", die meisten Refactorings fangen damit an, nicht ohne Grund.
Natürlich ist KISS auch sehr sehr wichtig.

Bin jetzt nicht überrrascht schalentier, dass du so hohe Ansprüche an Code hast, geht mir sehr ähnlich und kann doch da sehr gut nachvollziehen.
Was mir so im Alltag begegnet ist nicht selten jenseits von Gut & Böse, oft nur "geht so" bzw. "ich weiss das er das besser kann", wäre schon glücklich wenn meine Kollegen zumindest diese eine Regel einhalten würden, und natürlich keine Probleme lösen die nur in den Köpfen der Programmierer existieren, den letzteres hat nicht nur einmal zum Scheitern eines Projektes geführt...

 

[SlaterB]

'kein Code doppelt' bleibt meine Maxime, damit kommt in Untermethoden was darin gehört,
aber 100 individuelle Codezeilen bleiben in einer Methode hintereinander,
es sei denn 10 davon werden irgendwann woanders benötigt, dann wieder Methode..

 

[schalentier]

Der Punkt ist aber, dass es sich hier nicht "direkt" um Refactoring handelt, sondern darum, wie der Code von Anfang auszusehen hat

Okay, das stimmt natuerlich.

--offtopic--
Bin jetzt eher von meiner Perspektive ausgegangen und da seh ich eigentlich immer das Problem, dass eine (groessere) Software von einem jungen Team (nichts gegen junge Entwickler :-D) begonnen wurde (mit hohen Zielen und viel Ehrgeiz), aber mit wenig Erfahrung. Im Ergebnis gibt es dann Codemonster mit Tausenden Zeilen, krankesten Vererbungshierarchien (ich seh Vererbung zunehmend als Codesmell; sorry, da kommt bissel Radikalismus durch ;-) ) und irgendwelchen Cachealgorithmen, die im besten Fall einfach nur Code aufblaehen, im schlimmsten Fall wie eine Handbremse wirken. Kurz bevor das Projekt dann an den Nagel gehangen wird, holt man einen Berater, der dann aber oft auch nicht mehr wirklich viel machen kann.

Dafuer koennen die Entwickler aber nichts, eher das Management, was eben lieber noch 10 weitere, preiswerte Anfaenger einstellt, als wenigstens einen teuren aber erfahrenen.

Das einzige was dann auf Dauer gesehen hilft, ist mit gutem Beispiel voranzugehen und den Leuten zu beweisen, dass es durch einfachere Strukturen, viiiel kuerzer Methoden, Unit-/Integrationstests und Weiterbildungen aller Art (z.B. Pragmatischen Programmierer + Clean Code zur Pflichtlektuere erheben), wirklich besser wird.
--ende--

Interessant ist nun die Frage, wie man genau das an die "Anfaenger" herantraegt. Wenn ich mich an mein Studium zurueckerinnere, da gings in den Softwaretechnologievorlesungen eigentlich immer um Designpatterns oder irgendwelche Algorithmen. Oder halt nen Einsteigerkurs in C++ oder Java - der meistens von einem hoehersemestrigen Studenten gehalten wurde. Imho muesste man hier viel mehr gute Leute aus der Industrie mit echten Erfahrungen an die Unis/FH schicken, um dort "CleanCode"-Pflichtveranstaltungen zu halten.

Also im Grunde stimme ich dir zu, die Regel koennte man als zentrales Element auffassen. Allerdings reicht das Aufstellen der Regel wohl nicht. Man muss auch begruenden, wieso und warum und vor allem, was passiert wenn man sich nicht an sie haelt. Zudem muessen die kurzen Methoden auch sinnvoll und passend sein. Auch in einem 5 LOC Getter kann man z.B. nen fiesen Seiteneffekt einbauen, der in einem Getter NICHTS zu suchen hat

Edit: In meinen Augen wird in der IT Ausbildung auch seit jeher vernachlaessigt, dass man sich bestehenden Code ansieht und versteht. Das macht man in jedem anderen Beruf wie selbstverstaendlich, nur in der IT ist das irgendwie sehr selten anzutreffen...

 

[MrWhite]

Mir ist eine Regel natürlich zu wenig und deswegen mache ich 2 draus:
1. Methoden haben kurz zu sein.
2. Methoden haben noch kürzer zu sein!
(frei nach Bob Martin)

Das ist schön und gut. Aber ich erinnere an Paracelsus:

Alles ist ein Gift, es kommt nur auf die Menge an.

Methoden haben so groß zu sein, wie sie sein müssen.

Das können durchaus manchmal 20 Zeilen, in Ausnahmefällen sogar manchmal 50 sein. Was soll ich die Code-Datei derart fragmentieren? Beim Debugging sind mehrere kleinere Methoden auch nicht immer hilfreich, teilweise sogar nervig.

Diese Regel hat sicherlich ihre Daseinsberechtigung, aber ihr dogmatisch zu folgen ist trotzdem falsch. Es gibt durchaus Fälle, wo eine längere Methode die bessere Wahl ist.

 

[JohannisderKaeufer]

Beim Debugging

Vielleicht bin ich ja noch etwas zu Jung, aber Debugging?

Debugging halt ich eher für ein Zeichen von schlecht testbarem Code, bzw. aus Ermangelung irgendwelcher Gründe nicht getestetem Code.

Und mit Testen meine ich automatisches Testen, JUnit, Spock, Mocks, etc.

Naja, und für 200-Zeilen-Methoden lassen sich in der Regel nur sehr schlecht Tests schreiben, was dazu führt, das man hier um einen Debugger nur schwer rumkommt, wenn man nicht Refactoren will.


Darüber hinaus empfiehlt es sich auch eine möglichst umfassende Testsuite zu haben, bevor man mit einem Refactoring anfängt.


Aber, Debugging mit Debugger, das halte ich für ein Relikt aus "hoffentlich" längst vergangenen Zeiten.

 

[Marcinek]

Debugging halt ich eher für ein Zeichen von schlecht testbarem Code, bzw. aus Ermangelung irgendwelcher Gründe nicht getestetem Code.

Hört sich für mich an, wie "Nachts ist es kälter als Draußen".

Das Debugging ist ein Mittel zur Verifizierung deines Codes, nachdem du ihn entwickelt hast.

Wenn du hier Einen Code, der 100 Zeilen hat schon 5 (wohl dann private -Methoden) hast, dann bin ich gespannt, wie du diese testen wirst.

In jeder Zeile schon mal eine Debugausgabe und dann vollständige Objekte zurückgeben?


Und was passiert, wenn nun dein Test eine Exception ergibt?

 

[DanZ]

Kann ich nur bestätigen, in meinem Arbeitsalltag hat der Debugger trotz all den modernen Prinzipien eine feste Rolle sind ja auch vollkommen andere Einsatzgebiete. Tests setz ich ein um ... naja halt sicherzustellen, dass alles richtig läuft. Den Debugger setz ich ein, wenn es nicht richtig läuft aber ich nicht auf den ersten Blick sehe warum.

 

[tfa]

Das Debugging ist ein Mittel zur Verifizierung deines Codes, nachdem du ihn entwickelt hast.

Wohl eher zum Finden von Bugs, die die Tests übersehen haben. Das sollte aber nicht so oft nötig sein, wenn man vernünftig programmiert und getestet hat. Es sei dann, man hat die berühmten 100- oder 500-zeiligen Legacy-Methoden. In Zeiten von Frameworks, die AOP einsetzen, ist Debugging auch wirklich kein Spaß mehr.

Wenn du hier Einen Code, der 100 Zeilen hat schon 5 (wohl dann private -Methoden) hast, dann bin ich gespannt, wie du diese testen wirst.

Die 100 Zeilen und 5 Methoden fallen doch nicht einfach plötzlich vom Himmel. Wenn man TDD betreibt, entstehen die schrittweise, werden getestet, refaktoriert und wieder getestet.

 

[bygones]

wer debuggen als test nutzt hat testen nicht verstanden.

schon allein das argument der automatisierung ist es, was debuggen unsinnig und zeitverschwendet werden laesst

 

[schalentier]

Das ist doch wieder genau diese Kluft zwischen Theorie und Realitaet.
Natuerlich, wenn ich ein Projekt habe, welches gut designed, oft refaktored und komplett testdriven entwickelt wurde, dann braucht man tatsaechlich keinen Debugger, denn alleine der Name des fehlschlagenden Tests weisst mich direkt auf die defekte Codestelle.

In der Realitaet sind solche Projekt aber extrem selten anzutreffen. Wenn man Glueck hat, gibt es einen (Integrations-)Test, der eben fehlschlaegt mit der Aussage: "Irgendwas ist falsch". Dann ist ein Debugger sehr hilfreich und eine sinnvolle Alternative zu endlos vielen Debug-Ausgaben. Zumal ein guter Debugger auch nur wenig Probleme mit Frameworks und AOP haben sollte. Weiterhin finde ich den Debugger extrem nuetzlich, wenns darum geht fremden Code (mit wenig Tests) zu verstehen.

Trotzdem stimmt es natuerlich, die Zeit in der man Code zu debuggt, haette man auch sinnvoller mit dem Schreiben eines Tests nutzen koennen. Oftmals ist es aber einfach nicht moeglich, in akzeptabler Zeit und mit halbwegs ertraeglichem Aufwand, einen Test fuer einen Bug in einem Legacy Projekt (welches nicht fuers Testen geschrieben wurde) zu erstellen.

 

[Marcinek]

Ich habe gerade mein Posting nochmal durchgelesen: Was ich eigentlich sagen möchte ist, dass Debugging und Test eigentlich nix miteinander zu tun haben.

Man kann das Testen nicht durch debuggen ablösen noch kann man das debuggen durch tests ablösen ;D

Sind einfach zwei verschiedene Anwendungsgebiete.

 

[maki]

Methoden haben so groß zu sein, wie sie sein müssen.

Das können durchaus manchmal 20 Zeilen, in Ausnahmefällen sogar manchmal 50 sein. ...

Wie groß "müssen" denn Methoden sein?

Die einzigen Spielverderbder die ich kenne bez. der "Methoden haben kurz zu sein" Regel sind GUI Code (wenn man gleichzeitig die Komponenten erzeugt, das Layout in Java Code setzen muss und noch verhalten reinbringt, bei deklerativen UI ist das anders weil SoC beachtet wird ) und math. Algorithmen.

Ansonsten können Methoden Prinzipiell von 1 Zeile bis 5000 Zeilen lang sein, da gibt es kein "müssen".
"Müssen" gibt es erst, wenn man bestimmte Regeln einzuhalten hat.

Wenn diese Regeln lauten SRP, SoC, DRY, keine Inline Kommentare, sprechende Namen, also das was man als "sauberen Code" bezeichnet, werden die Methoden zwangsläufig kurz werden.

Wenn man nun alle die komplexen Regeln weglässt und stattdessen festlegt dass Methoden kurz zu sein haben, bekommt man sehr ähnliche Ergebnisse wie mit den komplexen Regeln, nur eben durch eine einzige einfache Regel.

Zu sagen dass Methoden (Ausnahmen aussen vor) 50 Zeilen lang sein müssen klingt für mich ehrlich gesagt nur so als ob jemand nicht weiss wie man Methoden aufsplittet. Die Wahrheit ist aber wohl, das Leuten die Motivation fehlt ihre Methoden kurz zu halten.

Was soll ich die Code-Datei derart fragmentieren?

Du hast strukturieren falsch geschrieben Mr. White

Beim Debugging sind mehrere kleinere Methoden auch nicht immer hilfreich, teilweise sogar nervig.

Debuggen ist immer nervig, sehe aber nicht warum das wichtiger sein sollte als lesbasren Code zu haben, und kurze Methoden mit aussagefähigen Namen sind immer lesbarer als lange Methoden die "zu viel machen".

Viele Probleme mit Design haben ihre Ursache darin, dass man an einer Stelle "zuviel macht".

Wenn man zB. alle temporäre Variablen mit Methodenaufrufen ersetzt schrumpfen die Methoden (und Klassen) von selber. Inline Kommentare spart man sich dadurch so sowieso.

Diese Regel hat sicherlich ihre Daseinsberechtigung, aber ihr dogmatisch zu folgen ist trotzdem falsch. Es gibt durchaus Fälle, wo eine längere Methode die bessere Wahl ist.

Mag sein, aber das sind Ausnahmen ime

 

[maki]

@Marcinek

Das Debugging ist ein Mittel zur Verifizierung deines Codes, nachdem du ihn entwickelt hast.

Das ist Debugging sicherlich nicht.

@schalentier

Das ist doch wieder genau diese Kluft zwischen Theorie und Realitaet.

Regeln beziehen sind immer auf "Theorie", und irgendwie muss man ja Erfahrungen aus der Praxis wieder in die theo. Fundamente einfliessen lassen.

Persönlich nutze ich den Debugger nur wenn ich verzweifelt bin, das ist in Legacy Projekten öfters der Fall (würg), in Greenfield Projekten selten.
Aber ums Debnuggen ging es hier gar nicht erst
Ums testen zwar auch nciht, aber testen wird einfacher mit kleinen Klassen/Methoden die nur eine Sache machen.

Habe schon oft gesehen wie man Uni Abgängern ein Greenfield Projekt anvertraut hatte, einmal weil sie billiger waren, dann weil sie so motiviert genug waren dass sie wirklich versuchten jede schwäche des Projektmanagements und Anforderungsmanagements durch überstunden zu kompensieren. Was dabei rauskommt ist aber klar... SW Entwickler, speziell Javaentwickler sind nunmal keine Waschbeckenmonteure IMHO... da gehört schon mehr dazu.

 

[MrWhite]

Wie groß "müssen" denn Methoden sein?

So groß wie nötig, so klein, wie möglich.

Zu sagen dass Methoden (Ausnahmen aussen vor) 50 Zeilen lang sein müssen klingt für mich ehrlich gesagt nur so als ob jemand nicht weiss wie man Methoden aufsplittet. Die Wahrheit ist aber wohl, das Leuten die Motivation fehlt ihre Methoden kurz zu halten.

Klar, praktiziere ich hier. Ich fülle Methoden auch sehr erfolgreich mit Leerzeilen auf. :lol:

Du hast strukturieren falsch geschrieben Mr. White

Nee, nicht immer. Gerade nach dem SRP ists manchmal besser eine Methode nicht weiter zu fragmentieren. Ihr sagt zwar, dass ihr alles über Tests abdeckt, aber das glaube ich euch nicht. Ihr braucht bestimmt auch ziemlich oft den Debugger oder eure Projekte sind nicht sonderlich komplex. Wir haben hier am aktuellen Projekt 90% automatisierte Testabdeckung (via Unit- und UITests) und dennoch muss man öfter mal den Debugger benutzen. In solchen Fällen ist jeder zusätzliche Stackframe einfach nur nervig, man hat schon genug davon. Das geht soweit, dass man vor lauter Step-Into nicht mehr so einfach die Stelle mit dem Fehler finden kann.

Debuggen ist immer nervig, sehe aber nicht warum das wichtiger sein sollte als lesbasren Code zu haben, und kurze Methoden mit aussagefähigen Namen sind immer lesbarer als lange Methoden die "zu viel machen".

Da stimme ich nicht zu. Eine lange Methode ist nicht grundsätzlich schwer lesbar. Eine lange, schlecht geschriebene und komplexe Methode ist schwer lesbar. Manchmal muss man auch Performance-Tweaks durchführen, das führt zu notwendigem, aber oft schlecht lesbarem Code.

Viele Probleme mit Design haben ihre Ursache darin, dass man an einer Stelle "zuviel macht".

Binsenweisheit.

Wenn man zB. alle temporäre Variablen mit Methodenaufrufen ersetzt schrumpfen die Methoden (und Klassen) von selber. Inline Kommentare spart man sich dadurch so sowieso.

Kann man nicht generell sagen. Vor allem die Aussage, dass dann auch die Klassen schrumpfen ist bestimmt nicht generell gültig. Ich kann mir einige Fälle vorstellen, in denen du nach so einem Refactoring in einer Klasse mehr LoCs als davor hättest.

 

[maki]

Nee, nicht immer. Gerade nach dem SRP ists manchmal besser eine Methode nicht weiter zu fragmentieren.

Sehe ich ganz anders, wie sonst willst du SRP erreichen, also nur eine einzige Sache machen, und die dafür gut?
Wieder benuztzt du "fragmentieren", und halte wiedermal "strukturieren" dagegen, "teile und herrsche" ist ja nix neues.

Zu sagen das der Code fragmentiert würde wenn man ihn runterbricht würde nur implizieren dass man nicht weiss wie man Code strukturiert, diesen Eindruck habe ich oft wenn ich "hingespuckten" Code sehe.

Ihr sagt zwar, dass ihr alles über Tests abdeckt, aber das glaube ich euch nicht. Ihr braucht bestimmt auch ziemlich oft den Debugger oder eure Projekte sind nicht sonderlich komplex. Wir haben hier am aktuellen Projekt 90% automatisierte Testabdeckung (via Unit- und UITests) und dennoch muss man öfter mal den Debugger benutzen. In solchen Fällen ist jeder zusätzliche Stackframe einfach nur nervig, man hat schon genug davon. Das geht soweit, dass man vor lauter Step-Into nicht mehr so einfach die Stelle mit dem Fehler finden kann.

Weiss nicht Mr. White, in komplexen Projekten, mit komplexen Frameworks/API/Container bringt einem der Debugger hauptsächlich eine Sache: Verwirrung
In dyn. Proxies zu navigieren (AOP, Spring, Hibernate, etc.) mit einem Debugger ist sicherlich nicht etwas das man "öfters" machen sollte, dasselbe gilt für generierten bytecode wie mit Guice, EclipseLink, etc. pp.
Gerade in komplexen Projekten sind Debugger komplexer als sonstwas.

Ansonsten verstehe ich dein Problem mit dem Step-Into nicht, gibt doch nix besseres als wenn ich sagen kann "bis hierhin stimmt alles".

Da stimme ich nicht zu. Eine lange Methode ist nicht grundsätzlich schwer lesbar. Eine lange, schlecht geschriebene und komplexe Methode ist schwer lesbar. Manchmal muss man auch Performance-Tweaks durchführen, das führt zu notwendigem, aber oft schlecht lesbarem Code.

Ich habe noch nie eine lange Methode gesehen bei der ich auf den ersten Blick wusste was da pasiert.
Bei kurzen Methoden (3-6 Zeilen) geht mir das recht häufig so.
"Performance Tweaks"? Meine Meinung dazu solltest du kennen, IME ist es auch sehr selten dass man davon soviele braucht dass das Programm schlecht lesbar wird.
Wie dem auch so, man fängt ja nicht mit Performance Tweaks an und saubereren code zu optimieren ist einfacher als unstrukturierten.

Binsenweisheit.

Tja, eben ein Fakt den man nicht ignorieren sollte

Kann man nicht generell sagen. Vor allem die Aussage, dass dann auch die Klassen schrumpfen ist bestimmt nicht generell gültig. Ich kann mir einige Fälle vorstellen, in denen du nach so einem Refactoring in einer Klasse mehr LoCs als davor hättest.

Doch, kann man generell so sagen, hört sich jetzt für mich so an als ob du noch nie temp. Variablen durch Methdoenaufrufe ersetzt hättest, ist nämlich das allererste was einem ins Auge sticht bei diesem Refactoring.

Wenn man natürlich Methoden nie runterbricht und sie immer schön groß hält dann merkt man davon nix...

Verstehe auch die Furcht/Zurückhaltung was das runterbrechen betrifft nicht, dafür gibt es Tastenkürzel in Eclipse (Alt+Shift+M), umgekehrt geht das genauso (Alt+Shift+I), ist nix kompliziertes dran.

 

[Andgalf]

Ich habe noch nie eine lange Methode gesehen bei der ich auf den ersten Blick wusste was da pasiert.
Bei kurzen Methoden (3-6 Zeilen) geht mir das recht häufig so.

Hier stimme ich maki voll und ganz zu. Was eine kurze Methode macht ist erheblich schneller erfasst als bei einer langen. Liegt doch schon in der Natur der Sache, ob ich 3-6 oder 20 Zeilen code erfassen muss

Verstehe auch die Furcht/Zurückhaltung was das runterbrechen betrifft nicht, dafür gibt es Tastenkürzel in Eclipse (Alt+Shift+M), umgekehrt geht das genauso (Alt+Shift+I), ist nix kompliziertes dran.

Auch das stimmt. Wenn man hier die Möglichkeiten seiner IDE kennt und nutzt geht das "razfaz". Um diese kennenzulernen bieten sich übrigens Koding-Catas und Code-Retreats an.
Viele haben wahrscheinlich nicht die Möglichkeit das im eigenen Unternehmen bzw. während der Arbeitszeit durchzuführen. Aber auch dafür findet man usergroups.

Hier mal ein link zu den Softwerkskammern, die sind lokal über ganz Deutschland verteilt:
Das Softwerkskammer Wiki : Softwerkskammer : GroupSpaces

 

[Gast2]

Zuletzt ein Treffen mit 30 verschiedenen ProjektLeiter/Entwickler gehabt, die meisten haben 20% duplicated Code in ihren Projekten. Erschreckend.

Hier stimme ich maki voll und ganz zu. Was eine kurze Methode macht ist erheblich schneller erfasst als bei einer langen. Liegt doch schon in der Natur der Sache, ob ich 3-6 oder 20 Zeilen code erfassen muss

Im Endeeffekt wird man bei vielen kleinen Methoden mehr LOC haben.


Ich bin genauso für kurze, prägnante und sprechende Methoden und Methodennamen.
Es heißt schließlich Code lesen und nicht Code erraten.

 

[maki]

Im Endeeffekt wird man bei vielen kleinen Methoden mehr LOC haben.

Das würde ich so nicht unterschreiben
Man hätte zwar mehr Methodenrümpfe (also 2 Zeilen zusätzlich pro Methode), dafür spart man sich aber temp. Variablen (pro Variable eine Zeile) und die Inlinekommentare, die erklären sollen was gerade passiert (u.U. mehrere Zeilen weniger).
Wenn man jetzt noch die eingsparten Redundanzen wegrechnet (direkte und "indirekte") wundert es einen nicht mehr... sieht man aber wie gesagt sehr schnell in der Praxis.

 

[Luk10]

Hallo,

Ich hab die Beträge hier durchgelesen und bin daran interessiert bei meinem (zukünftigem) Code mehr auf "Einfachheit" zu achten. Was ich aber nicht verstehe ist das hier:

[...] dafür spart man sich aber temp. Variablen (pro Variable eine Zeile) [...]

Kann mir das jemand erklären oder ein kurzes Beispiel posten? Kann ich mir nicht vorstellen wie das funktionieren soll.

Außerdem stellts sich mir die Frage bis zu welchem Grad man Methoden aufteilen soll. Wie erkenne ich, dass eine Methode mehr als nur eine Aufgabe erfüllt? Ist es dann sinnvoll sie aufzuteilen?

Danke,
-Luk10-

 

[Fu3L]

Ohne die extra Methode müsste man eine temporäre Variable anlegen:
(Besseres Beispiel habe ich bei mir so schnell nicht gefunden)

//Irgendwo
if(isResultValid(cr) && dist < cDist) {


private boolean isResultValid(CollisionResult cr) {
		if(overview) {
			return cr.getGeometry().getName().startsWith("marker_");
		} else {
			return !cr.getGeometry().getName().startsWith("shape_") && !cr.getGeometry().getName().startsWith("highlight_");
		}
	}

 

[Luk10]

Hm also so ganz klar ist mir das noch nicht:

Ich habe z.B. folgende Methode, die mir RGB-Farbe

newIn

über RGB-Farbe

oldIn

legt:

private int computeColor(int oldIn, int newIn) {

	// newIn over oldIn
	int result = 0;

	// {Red, Green, Blue, Alpha}
	int[] oldRgb = { (oldIn >>> 16) & 0xff, (oldIn >>> 8) & 0xff, (oldIn >>> 0) & 0xff, (oldIn >>> 24) };
	int[] newRgb = { (newIn >>> 16) & 0xff, (newIn >>> 8) & 0xff, (newIn >>> 0) & 0xff, (newIn >>> 24) };

	// Values between 0.0 and 1.0
	double[] resultArgb = { -1, -1, -1, -1 };

	resultArgb[3] = (newRgb[3] / 255.0) + (1 - (newRgb[3] / 255.0)) * (oldRgb[3] / 255.0);
	result = (result | (int) Math.round(resultArgb[3] * 255.0)) << 8;

	for (int i = 0; i < 3; i++) {

	    resultArgb[i] = (1.0 / resultArgb[3]) * ((newRgb[3] / 255.0) * (newRgb[i] / 255.0) + (1 - (newRgb[3] / 255.0)) * (oldRgb[3] / 255.0) * (oldRgb[i] / 255.0));
	    result |= (int) Math.round(resultArgb[i] * 255.0);
	    if (i < 2) result <<= 8;

	}

	return result;

}

Das sind 25 Zeilen, ohne Kommentar und Leerzeilen 13. Die Methode braucht 3 lokale Variablen und ruft keine Weiteren Methoden (Außer

Math.round()

) auf.

Könnte man das hier "sauberer" umsetzten? Verlange nicht, dass ihr mir das jetzt macht, nur Denkanstöße ...

Luk10

 

[JohannisderKaeufer]

Zeile 7 und 8 sehen doch schon sehr sehr ähnlich aus oder?

oldIn und newIn werden nach zeile 8 überhaupt nicht mehr verwendet.

int result = 0; wird in Zeile 4 initialisiert und erst in 14 das erste mal verwendet.

Daher würde ich Zeile 14 in das ändern
int result = (0 | (int) Math.round(resultArgb[3] * 255.0)) << 8;
Wenn man sich irgendwann mal Zeile 14 anschaut, weil da etwas Mysteriöses passiert, dann fragt man sich ob es an dem Wert von result liegen kann. So wie es jetzt ist mußt du die Stelle finden, an der result initialisiert wird (Zeile 4) und dann alle Zeilen (bis 14) durchgehen, um zu sehen, das mit result garnichts gemacht wurde.

Wie erkenne ich, dass eine Methode mehr als nur eine Aufgabe erfüllt?

Ein gutes Indiz dafür ist, dass man Kommentare im Quelltext verwendet, anstatt JavaDocs an Methoden. (Zeile 3, 6, 10)

Das Beispiel von Fuel würde anders so aussehen

boolean isResultValid = false;
if(overview) {
            isResultValid = cr.getGeometry().getName().startsWith("marker_");
} else {
            isResultValid = !cr.getGeometry().getName().startsWith("shape_") && !cr.getGeometry().getName().startsWith("highlight_");
}


//Irgendwo
if(isResultValid && dist < cDist) {

Also 10 Zeilen, man spart sich die Methodensignatur und eine schließende Klammer, benötigt aber eine temporäre Variable (Zeile 1)

Bei dem Beispiel von Fuel könnte man für die Doku JavaDoc an der Methode verwenden. Hier bräuchte man Inline Kommentare.

 

[Antoras]

//Irgendwo
if(isResultValid(cr) && dist < cDist) {


private boolean isResultValid(CollisionResult cr) {
		if(overview) {
			return cr.getGeometry().getName().startsWith("marker_");
		} else {
			return !cr.getGeometry().getName().startsWith("shape_") && !cr.getGeometry().getName().startsWith("highlight_");
		}
	}

Damit wäre ich nicht zufrieden.

private boolean isResultValid(CollisionResult cr) {
  return overview ? startsWithName(cr, "marker_") : !startsWithName(cr, "shape") && !startsWithName(cr, "highlight_");
}

private boolean startsWithName(CollisionResult cr, String name) {
  return cr.getGeometry().getName().startsWith(name);
}

 

[Landei]

Eine Duplikation in obigem Beispiel könnte man mit

private static int[] rgbToArray(int rgb) { 
    return new int[]{ (rgb >>> 16) & 0xff, (rgb >>> 8) & 0xff, (rgb >>> 0) & 0xff, (rgb >>> 24) };
}

beseitigen.

 

[Fu3L]

Danke Antoras. Hatte schon beim Posten gesehen, dass da bestimmt noch was geht, aber noch keine Zeit zum erneuten Draufgucken gehabt^^

 

[Landei]

Wie wäre...

private boolean isResultValid(CollisionResult cr) {
  return overview ? startsWithOneOf(cr, "marker_") : !startsWithOneOf(cr, "shape", "highlight_");
}
 
private boolean startsWithOneOf(CollisionResult cr, String... names) {
  for(String name : names) 
     if (cr.getGeometry().getName().startsWith(name)) return true;
  return false;
}

 

[JohannisderKaeufer]

return overview ? startsWithOneOf(cr, "marker_") : !startsWithOneOf(cr, "shape", "highlight_");

Wenn auf den Tertiär-Operator verzichtet werden soll, kann man auch zu diesem hier gelangen.

return (overview && startsWithOneOf(cr, "marker_")) || (!overview && !startsWithOneOf(cr, "shape", "highlight_"));

Aber bevor ich es noch vergesse.

Wenn man overview nicht als Boolean deklariert, sondern als ein State. Dann kann man sich auch von der ein oder anderen Verzweigung (if-else, oder Tertiäroperator) trennen.

public abstract class State{
  public abstract boolean isResultValid(CollisionResult cr);
  
  protected final boolean startsWithOneOf(CollisionResult cr, String... names) {
    for(String name : names) 
       if (cr.getGeometry().getName().startsWith(name)) return true;
    return false;
  }
}


public class Overview extends State{
  public boolean isResultValid(CollisionResult cr){
    return startsWithOneOf(cr, "marker_");
  }
}

public class NotOverview extends State{
  public boolean isResultValid(CollisionResult cr){
    return !startsWithOneOf(cr, "shape", "highlight_");
  }
}

Ein Umsetzung als Enum wäre hier auch denkbar.

Das sieht jetzt erstmal nach viel mehr Code aus, was es auch ist. Aber Overview und NotOverview sind einzeln testbar.

Außerdem kann man es direkt auf einem "state" anwenden.
... state.isResultValid(cr);

Bei der Version mit overview als boolean hat man das Problem, dass overview entweder irgendwo global, definiert werden muß, oder es muß als weiterer Parameter, einer Methode mit übergeben werden.

 

[Fu3L]

Das mit den States gefällt mir. Werde ich mir auf jeden Fall merken, wenn ich den Editor das nächste Mal verbessere und erweitere. Da können dann noch weitere Methode zum Umschalten der States rein, was die Hauptklasse wieder etwas kleiner macht (ich schäme mich nämlich heimlich für ihre Länge trotz mehrfachem Auslagerns von Code in andere Klassen )

 

[maki]

Da können dann noch weitere Methode zum Umschalten der States rein, was die Hauptklasse wieder etwas kleiner macht

Kannst dir mal ansehen, welche Methoden welche Attribute nutzen (sog. Koheränz, also wie stark der "innere zusammenhalt" ist).
Wenn du zB. 5 Methoden hast, die alle 4 Attribute verwenden die sonst nirgendwo verwendet werden, könnte man da eine neue Klasse auslagern.

Das macht man zB. wenn man die Methoden schon recht kurz hat und nun die Klasse selber aufspalten will.

 

[timbeau]

Aber ist das so viel besser? Performancetechnisch wird doch sicher eh irgendwo eine temporäre Variable angelegt oder?

Und lesbarer finde ich es nicht.

 

[Fu3L]

Ich glaube nicht, dass die Designrichtlinien, die insbesondere Maki hier predigt, auf Performance abzielen. Wenn ich nie eine Methode über 10 Zeilen schreiben dürfte, müsste ich an einigen Stellen Objekte erstellen, um die Zwischenergebnisse zu übergeben. Normal mag das gut sein, aber nicht wenns ein Loop über alle bewegten Objekte indirekt innerhalb des GameLoops ist^^

Was ich aber schön finde, wenn man sich an sowas hält: Man kann eine Methode dann fast wie richtigen Text lesen und braucht nicht ewig darüber zu brüten was sie wie tut^^ (Insbesondere wenn die Kommentierung mau ist)

 

[maki]

Aber ist das so viel besser? Performancetechnisch wird doch sicher eh irgendwo eine temporäre Variable angelegt oder?

Seit wann ist denn Performance relevant?
Hier geht es um sauberen Code bzw. Design.
Dieser lässt sich einfacher optimieren falls nötig.
Nebenbei bemerkt, der JIT/HotSpot Compiler kann "kurze Methoden" besser optimieren, ist aber wie gesagt erstmal gar nicht relevant

Und lesbarer finde ich es nicht.

Natürlich ist es lesbarer, sieht man am besten am Beispielen aus der Realität.
Wie Fu3L sagte liest man weniger Implementierungsdetails (also das wie) sondern mehr (hoffentlich) sprechende Methodennamen (also das was).

 

[Luk10]

Ich hab jetzt probiert meine Alpha-Blending-Methode zu verbessern ... würde gerne wissen was ihr davon haltet.

Alt:

private int computeColor(int oldIn, int newIn) {

	// newIn over oldIn
	int result = 0;

	// {Red, Green, Blue, Alpha}
	int[] oldRgb = { (oldIn >>> 16) & 0xff, (oldIn >>> 8) & 0xff, (oldIn >>> 0) & 0xff, (oldIn >>> 24) };
	int[] newRgb = { (newIn >>> 16) & 0xff, (newIn >>> 8) & 0xff, (newIn >>> 0) & 0xff, (newIn >>> 24) };

	// Values between 0.0 and 1.0
	double[] resultArgb = { -1, -1, -1, -1 };

	resultArgb[3] = (newRgb[3] / 255.0) + (1 - (newRgb[3] / 255.0)) * (oldRgb[3] / 255.0);
	result = (result | (int) Math.round(resultArgb[3] * 255.0)) << 8;

	for (int i = 0; i < 3; i++) {

	    resultArgb[i] = (1.0 / resultArgb[3]) * ((newRgb[3] / 255.0) * (newRgb[i] / 255.0) + (1 - (newRgb[3] / 255.0)) * (oldRgb[3] / 255.0) * (oldRgb[i] / 255.0));
	    result |= (int) Math.round(resultArgb[i] * 255.0);
	    if (i < 2) result <<= 8;

	}

	return result;

}

Neu:

private int computeBlendedColor(int oldColor, int newColor) {

	int result = 0x0;
	double resultAlpha = computeBlendedAlpha(colorToRgba(oldColor, 0), colorToRgba(newColor, 0)) / 255.0;
	result |= Math.round(resultAlpha * 255.0) << 8;
	result |= computeBlendedRgb(colorToRgba(oldColor, 0), colorToRgba(oldColor, 1), colorToRgba(newColor, 0), colorToRgba(newColor, 1), resultAlpha) << 8;
	result |= computeBlendedRgb(colorToRgba(oldColor, 0), colorToRgba(oldColor, 2), colorToRgba(newColor, 0), colorToRgba(newColor, 2), resultAlpha) << 8;
	result |= computeBlendedRgb(colorToRgba(oldColor, 0), colorToRgba(oldColor, 3), colorToRgba(newColor, 0), colorToRgba(newColor, 3), resultAlpha);
	return result;

}

private int computeBlendedAlpha(double oldAlpha, double newAlpha) {

	return (int) Math.round((newAlpha + (1 - newAlpha) * oldAlpha) * 255.0);

}

private int computeBlendedRgb(double oldAlpha, double oldRgb, double newAlpha, double newRgb, double resultAlpha) {

	return (int) Math.round(((1.0 / resultAlpha) * newAlpha * newRgb + (1 - newAlpha) * oldAlpha * oldRgb) * 255.0);

}

public static double colorToRgba(int color, int channel) {

	return ((color >>> (24 - channel * 8)) & 0xff) / 255.0;

}

IMO ist es schon übersichtlicher geworden, wobei mir die 5! Argumente aus computeBlendedRgb(...) nicht besonders gefallen ... aber das in ein Array zu packen macht ja auch nicht besonders viel Sinn.

-Luk10-

 

[Antoras]

computeBlendedRgb(colorToRgba(oldColor, 0), colorToRgba(oldColor, 3), colorToRgba(newColor, 0), colorToRgba(newColor, 3), resultAlpha);

Das würde ich auch noch in eine Methode auslagern. Dann kannst du am Schluss schreiben:

double result = r1 | r2 | r3 | r4 | r5;

Wobei vernünftige Namen für die Variablen zu wählen sind, damit man am Schluss weiß was berechnet wurde.

Und vier mal

colorToRgba(oldColor, 0)

zu berechnen ist mit Sicherheit auch unnötig.

 

[Luk10]

Und vier mal colorToRgba(oldColor, 0) zu berechnen ist mit Sicherheit auch unnötig.

Sollte man dann hier eine Lokale Variable einführen? Ich dachte genau das sollte vermieden werden.

Danke für dein Feedback!

 

[Antoras]

Lokale Variablen sind nicht per se schlecht, sondern nur dann wenn sie unnötigerweise mutable sind, wenn sie ausgelagert werden können oder wenn sie den Code nicht weiter dokumentieren. Kurz: Vermeide Statements und nutze Expressions.

Ersteres kann man sicherstellen wenn man

final

vor jede Variable schreibt. Immutable Code ist einfacher nachzuvollziehen und stellt sicher, dass eine Variable auch tatsächlich dafür verwendet wird wofür sie steht. Mein Beispiel

double result = r1 | r2 | r3 | r4 | r5;

fällt in diese Kategorie. Ihr wird ein Wert zugewiesen, der Wert wird in Form eines Variablennamens und -typ dokumentiert und sie repräsentiert diesen Wert bis zu ihrem Tod wenn sie mit

final

gekennzeichnet wurde.

Auslagern kann man prinzipiell jedes Statement, das mehr als einmal vorkommt. Das hast du ja bereits gut gemacht. Auch hier gilt wieder, dass der Methodenname und auch der Variablenname, an den der Rückgabewert der Methode gebunden wird, zur Dokumentation dienen.

Variablen, die den Code nicht weiter dokumentieren sind unnötig:

int getResult() {
  int result = // calc result
  return result;
}

Diese Variable ist unnötig, da sie keinen Mehrwert bietet.

In deinem Fall kann ein

double r1 = ...
double r2 = ...
double r3 = ...

sinnvoll sein, wenn die Variablennamen gut gewählt sind (nicht wie bei mir) und beschreiben was sie beinhalten. Variablennamen wie

r1, r2, r3

sind unnötig, da sie wieder keinen Mehrwert bieten. Weiterhin kann Effizienz eine Rolle spielen. Wenn es lange dauert das Ergebnis zu berechnen, ist es sinnvoll dieses zu cachen. Aber auch hier gilt wieder, dass die Variable dann auch so gekennzeichnet werden muss.

 

[JohannisderKaeufer]

Sollte man dann hier eine Lokale Variable einführen? Ich dachte genau das sollte vermieden werden.

Das muß man differenziert sehen.
Mehrmals, das selbe berechnen spricht gegen die Performanz.
Lokale Variablen einführen, gegen die Lesbarkeit, mit all seinen Folgen. Ausserdem neigen sie dazu sich zu vermehren.

Ein Kompromiss der beide Seiten berücksichtigt könnte so aussehen, indem man eine Klasse hat, die eine Farbe kapselt und die rgba-Werte cacht. Die Kosten lägen hier bei der Objekterzeugung.

public class Color{
  private final int color
  private final double[] rgbaValues = new double[4];

  public Color(int color){
    this.color = color;
    for(int i = 0; i<4;i++){
      rgbaValues[i] = colorToRgba(i);
    }
  }

  public double getRgba(int channel){
    return rgbaValues[channel];
  }
  
  private double colorToRgba(int channel) {
     return ((color >>> (24 - channel * 8)) & 0xff) / 255.0;
   }
}

und die Verwendung könnte dann so aussehen.

  private int computeBlendedColor(int oldColor, int newColor) {
    Color old = new Color(oldColor);
    Color new_ = new Color(newColor);
    return computeBlendedColor(old, new_);
 }
  private int computeBlendedColor(Color oldColor, Color newColor) {
    int result = 0x0;
    double resultAlpha = computeBlendedAlpha(oldColor.getRgba(0), newColor.getRgba(0)) / 255.0;
    result |= Math.round(resultAlpha * 255.0) << 8;
    result |= computeBlendedRgb(oldColor.getRgba(0), oldColor.getRgba(1), newColor.getRgba(0), newColor.getRgba(1), resultAlpha) << 8;
    result |= computeBlendedRgb(oldColor.getRgba(0), oldColor.getRgba(2), newColor.getRgba(0), newColor.getRgba(2), resultAlpha) << 8;
    result |= computeBlendedRgb(oldColor.getRgba(0), oldColor.getRgba(3), newColor.getRgba(0), newColor.getRgba(3), resultAlpha);
    return result;
 
}

Compute BlendedColor bleibt frei von Optimierungsdetails.

Da man nun ein ColorObjekt hat, kann man auch die Prameter für computeBlendedRbg deutlich reduzieren.

computeBlendedRgb(oldColor.getRgba(0), oldColor.getRgba(3), newColor.getRgba(0), newColor.getRgba(3), resultAlpha)
computeBlendedRgb(oldColor, newColor, channel, resultAlpha)

//was im Endeffekt dazu führt
  private int computeBlendedColor(Color oldColor, Color newColor) {
    int result = 0x0;
    double resultAlpha = computeBlendedAlpha(oldColor.getRgba(0), newColor.getRgba(0)) / 255.0;
    result |= Math.round(resultAlpha * 255.0) << 8;
    result |= computeBlendedRgb(oldColor, newColor, 1, resultAlpha) << 8;
    result |= computeBlendedRgb(oldColor, newColor, 2, resultAlpha) << 8;
    result |= computeBlendedRgb(oldColor, newColor, 3, resultAlpha);
    return result;
 
}

 

[Marco13]

Hatte diesen Thread eine Weile nicht mitverfolgt, aber jetzt nochmal reingeschaut. Ich wurschtle ja häufiger mal mit Farben/ARGB-Pixeln rum. Ohne jetzt verifiziert zu haben, ob das die richtige Implementierung einer der Porter-Duff-Regeln ist: Bei mir würde diese Methode wohl ungefähr so aussehen:

    private int computeColor(int argb0, int argb1) 
    {
        double a0 = ((argb0 >> 24) & 0xFF)/255.0;
        double r0 = ((argb0 >> 16) & 0xFF)/255.0;
        double g0 = ((argb0 >>  8) & 0xFF)/255.0;
        double b0 = ((argb0 >>  0) & 0xFF)/255.0;

        double a1 = ((argb1 >> 24) & 0xFF)/255.0;
        double r1 = ((argb1 >> 16) & 0xFF)/255.0;
        double g1 = ((argb1 >>  8) & 0xFF)/255.0;
        double b1 = ((argb1 >>  0) & 0xFF)/255.0;
        
        double a = a1 + (1-a1) * a0;
        double r = (1.0 / a) * ((a * r1) + (1 - a1) * a0 * r0);
        double g = (1.0 / a) * ((a * g1) + (1 - a1) * a0 * g0);
        double b = (1.0 / a) * ((a * b1) + (1 - a1) * a0 * b0);
        
        int result = 
            ((int)(a * 255) << 24) |
            ((int)(r * 255) << 16) |
            ((int)(g * 255) <<  8) |
            ((int)(b * 255) <<  0);
        return result;
    }

Bin mal gespannt, wie viele zustimmen, wenn ich sage, dass das durch das entstehende "Muster" sehr übersichtlich, leicht wartbar und nachvollziehbar ist (man sieht sofort: Erstmal die auf 0.0...1.0 normalisierten A,R,G,B vom ersten Farbwert holen, dann vom zweiten, dann werden sie verrechnet, und dann das Ergebnis zusammengebaut), und wie viele sagen, dass es ein mehrfaches Auftreten von [c]& 0xFF)/255.0[/c] ja eine Todsünde ist und es offenbar seeeeehr unterschiedliche Auffassungen von "clean Code" gibt...

In Anlehnung an den Eröffnungsbeitrag: Es sind zwar 13 Zeilen, aber wenn die Alternative dann darin besteht, sinnarme und schwer nachvollziehbare Hilfsmethoden einzuführen, die 5 oder mehr Parameter bekommen, ist für mich die Entscheidung klar ... (Wenige Methodenparameter würde ich auch als einen von vielen Aspekten von "Clean Code" ansehen, aber das nur nebenbei).

 

[Luk10]

Verstehe sowohl JohannisderKaeufers als auch Marco13s Ansatz.

Denke, dass Marco13s Methode, wie du auch selbst schon geschrieben hast, keine Hilfsmethoden braucht, was es IMO leichter nachzuvollziehen macht.

Ich kann aber aber überhaupt nicht mitreden wenn es darum geht welche nun "cleaner" ist.
Das müsst ihr schon ausdiskutieren

Danke aber für eure ausführliche Hilfe!!

-Luk10-

[EDIT]
So habs ich jetzt erstmal stehen. Denke mehr geht da [abgesehen von JohannisderKaeufers bzw. Marco13s Vorschlag] nicht.

private int computeBlendedColor(int oldColor, int newColor) {

	final double oldAlpha = colorToRgba(oldColor, 0);
	final double newAlpha = colorToRgba(newColor, 0);
	final double resultAlpha = computeBlendedAlpha(colorToRgba(oldColor, 0), colorToRgba(newColor, 0));
	
	return composeBlendedColor((int) Math.round(resultAlpha * 255.0) << 24,
		computeBlendedRgb(oldAlpha, colorToRgba(oldColor, 1), newAlpha, colorToRgba(newColor, 1) , resultAlpha) << 16,
		computeBlendedRgb(oldAlpha, colorToRgba(oldColor, 2), newAlpha, colorToRgba(newColor, 2), resultAlpha) << 8,
		computeBlendedRgb(oldAlpha, colorToRgba(oldColor, 3), newAlpha, colorToRgba(newColor, 3), resultAlpha)) << 0;

}
    
private int composeBlendedColor(int alpha, int red, int green, int blue) {
	
	return alpha | red | green | blue;
	
}

private double computeBlendedAlpha(double oldAlpha, double newAlpha) {

	return newAlpha + (1 - newAlpha) * oldAlpha;

}

private int computeBlendedRgb(double oldAlpha, double oldRgb, double newAlpha, double newRgb, double resultAlpha) {

	return (int) Math.round(((1.0 / resultAlpha) * newAlpha * newRgb + (1 - newAlpha) * oldAlpha * oldRgb) * 255.0);

}

public static double colorToRgba(int color, int channel) {

	return ((color >>> (24 - channel * 8)) & 0xff) / 255.0;

}

[/EDIT]

 

[JohannisderKaeufer]

Das von Marco13 sieht nicht schlecht aus. Sollte aber auch nicht automatisch formatiert werden.

Kann man aber durchaus vorstellen, dass eine eher Objektbasierte-Lösung auch ihren Reiz hat.
Hier ist zu beachten, daß die Klasse ARGBColor zwei implementierungsvarianten beinhaltet.

Der Vorteil, der sich hier ergibt, ist, daß das Aufsplitten von einem int-Wert in a,r,g,b und das berechnen der neuen Farbe deutlich getrennt (Konstruktor / Methode) ist und auch einzeln getestet werden kann. Bei einem Fehler (so unwahrscheinlich er auch sein mag) muss bei Marcos Variante ein Fehler in beiden Bereichen vermutet werden.

public class ARGBColor {
    
    private final double a,r,g,b;
    private final int value;

    public ARGBColor(int value){
        this.value = value;
        this.a = ((value >> 24) & 0xFF)/255.0;
        this.r = ((value >> 16) & 0xFF)/255.0;
        this.g = ((value >>  8) & 0xFF)/255.0;
        this.b = ((value >>  0) & 0xFF)/255.0;
    }

    //evtl. noch ein alternativer Constructor, der in dem Fall auch private sein kann
    public ARGBColor(double a, double r, double g, double b){
        this.a = a;
        this.r = r;
        this.g = g;
        this.b = b;

        this.value = ((int)(a * 255) << 24) |
            ((int)(r * 255) << 16) |
            ((int)(g * 255) <<  8) |
            ((int)(b * 255) <<  0);
    }

    public ARGBColor computeColor(ARGBColor c) 
    {
        double newA = c.a + (1-c.a) * a;
        double newR = (1.0 / newA) * ((newA * c.r) + (1 - c.a) * a * r);
        double newG = (1.0 / newA) * ((newA * c.g) + (1 - c.a) * a * g);
        double newB = (1.0 / newA) * ((newA * c.b) + (1 - c.a) * a * b);

        int result = 
            ((int)(newA * 255) << 24) |
            ((int)(newR * 255) << 16) |
            ((int)(newG * 255) <<  8) |
            ((int)(newB * 255) <<  0);
        return new ARGBColor(result);
    }

    public ARGBColor computeColorAlternativ(ARGBColor c2) 
    {
        double newA = c2.a + (1-c2.a) * a;
        double newR = (1.0 / newA) * ((newA * c2.r) + (1 - c2.a) * a * r);
        double newG = (1.0 / newA) * ((newA * c2.g) + (1 - c2.a) * a * g);
        double newB = (1.0 / newA) * ((newA * c2.b) + (1 - c2.a) * a * b);

        return new ARGBColor(newA, newR, newG, newB);
    }

    public int getValue(){
      return value;
    }
}

 

[Marco13]

Bei so zeitkritischen Dingen wie Bildverarbeitung bin ich mit Objekten und Methoden (TROTZ genialem JIT-Inlining und Escape Analysis) noch vorsichtig, und orientiere mich eher daran, wie unsere C-Vorfahren das vor 20 Jahren gemacht haben, um auch auf einem 386'er akzeptable Performance zu erreichen

EDIT: maki hatte zwar betont, dass Performance und Clean Code nichts miteinander zu tun haben, aber Bildverarbeitung ist IMMER zu langsam, deswegen kann man da die Prioritäten IMHO ein bißchen shiften.

 

[maki]

Die Lösung von JohannisderKaeufer gefällt mir am besten, ist OO, immutable, wenig redundanz (bis auf die Formel), SRP, SoC sind gut umgesetzt, minimale Anzahl an Methodenparametern (sind jetzt Instanzvariablen) und zeigt auch sehr schön wie man eine ganze Klasse extrahieren kann, Methoden und Daten sind eben in einer neuen Klasse zusammengefasst und gekapselt, können sehr einfach (wieder-)verwendet werden, alles in allem sehr verständlich und leicht zu warten.

EDIT: maki hatte zwar betont, dass Performance und Clean Code nichts miteinander zu tun haben, aber Bildverarbeitung ist IMMER zu langsam, deswegen kann man da die Prioritäten IMHO ein bißchen shiften.

Ja, ist immer die Frage was man braucht.

Die ganze Diskussion mit Clean Code/Design etc. bezieht sich auf OOP, Bereiche wie Grafik etc. haben aber eben die Anforderung an Performance, sind selten OO, meist Prozedural (sieht man an deiner Lösung), deswegen machen die keine guten Beispiele für solche Refactorings IMHO.
Entweder wir verzichten auf Beispiele auf der Bildverarbeitung, oder wir ignorieren mal für einen Moment die Performance, so wie man es bei nicht-Bildverarbeitungs SW macht (bzw. machen sollte).

Persönlich bin ich der Meinung dass man als Entwickler die Fähigkeit zu solchen Refactorings haben sollte, ob und wie man sie einsetzt hängt vom konkreten Fall ab.
"Unsauberen" Legacy Code aufzuräumen kommt ja im Berufsalltag recht häufig vor
Von daher kann man das nicht zuviel üben.

Wenn man erstmal wirklich extremistisch kurze Methoden schreiben bzw. raus-refaktorieren kann neigt man dazu dass so oft wie möglich so zu machen.
Den umgekehrten Weg, also mehrere Klassen/Methoden zu einer einzigen, großen Klasse/Methode zu refactoren ist auch ein legaler Weg (und manchmal auch nützlich), mit einer modernen IDE nur ein paar Klicks bzw. Tastenkürzel entfernt.

 

[schalentier]

Neben der Ergaenzung, dass im Code von JohannisderKaeufer die Zeilen 34-39 durch die Zeile 49 ersetzt werden koennen, wuerde ich noch anbringen, dass man die beiden Methoden computeXXX in zwei Klassen und ein Interface auslagern koennte. Das Interface wuerde dann z.B. ColorCombiner oder ColorMixer heissen und stellt eine Methode mixColors( ARGBColor a, ARGBColor b ) bereit - Implementierungen stellen dann verschiedene Algorithmen bereit, wie die beiden Farben konkret vermischt werden.

Der Vorteil waere, dass man dann z.B. eine andere Klassen 'MixImages' bauen koennte, die einen ColorMixer bekommt und eben ein komplettes Bild zusammenmischt.

Natuerlich ist das alles im Fall der Bildverarbeitung nicht besonders sinnvoll, wie bereits erwaehnt wuerde, aber es geht ja hier um guten Code ;-)

 

[Marco13]

Der Vorteil waere, dass man dann z.B. eine andere Klassen 'MixImages' bauen koennte, die einen ColorMixer bekommt und eben ein komplettes Bild zusammenmischt.

Natuerlich ist das alles im Fall der Bildverarbeitung nicht besonders sinnvoll

Konzeptuell ist das auf jeden Fall sinnvoll! Schon die spezifische ausprägung der "Kombinierregel" fand ich nicht so schön, und wenn man sich mal die vielen Porter-Duff-Regeln ansieht, die es gibt, wäre sowas sicher sinnvoll. Wenn man das auf einzelne Pixel anwenden will und Performance im Hinterkopf hat kommt dann zwar noch dazu, dass Polymorphe Aufrufe langsamer sind als Mono- oder Bimorphe, aber ... wieder so ein Punkt, wo man abwägen muss...