Umherspringende Methoden im quelltextbasierten Class Builder

Re: Umherspringende Methoden im quelltextbasierten Class Builder

rob_abc hat geschrieben: ↑

18.10.2024 21:42

Ich habe inzwischen aufgegeben und nutze ABAP Cleaner: https://github.com/SAP/abap-cleaner

Ich nutze zwar kein Eclipse, aber das sieht sehr gut aus - bis auf eine Sache, die ich auch hier immer mal wieder sehe und nicht nachvollziehen kann:
Stattdessen ist diese Einrückung sauberer und die alphabetische Sortierung lesbarer:

Re: Umherspringende Methoden im quelltextbasierten Class Builder

SELECTS gestalte ich grundsätzlich so:
Man beachte die doppelten Leerzeichen nach SELECT, INTO und FROM, die der vertikalen Ausrichtung zum WHERE dienen (wenn kein WHERE da ist, reicht natürlich 1 Leerzeichen). Bei JOINS wird die erste FROM-Tabelle zu den JOIN-Tabellen ebenfalls vertikal ausgerichtet ebenso wie zugehören ON-Bedingungen und etwaigen AS (was in dem Beispiel nicht genutzt wird). Das INTO schreibe ich immer direkt nach den zu selektierenden Feldern, weil das WHERE zum FROM gehört und das übliche SELECT FROM INTO WHERE oder SELECT FROM WHERE INTO da einen Bruch reinbringt, der mich persönlich stört.

Formatierungen sind aber allgemein von persönlichen Vorlieben und Angewohnheiten geprägt.

Re: Umherspringende Methoden im quelltextbasierten Class Builder

msfox hat geschrieben: ↑

19.10.2024 16:15

into corresponding fields of table @mt_einteilungen

Mir wurde mal beigebracht, dass das unperformant wäre.

INTO CORRESPONDING beeinträchtigt die Performance negativ bei:
- gepufferten Tabellen (hier sollte man einfach SELECT * nutzen)
- vielen Selektionsfeldern bei nicht-gepufferten Tabellen, die es auch in der Zielstruktur gibt (hier sollte man die konkreten Felder angeben, entweder im SELECT oder in der Typisierung der Zielstruktur oder beides)

Letzteres hat aber nichts mit INTO CORRESPONDING zu tun, sondern schlicht und ergreifend damit, dass mehr Daten gezogen werden.

Ein Fehlgedanke könnte dahingehend darin liegen, dass man meint, es würden (mit SELECT *) ohnehin alle Felder gezogen werden und mit INTO CORRESPONDING lediglich nachträglich wieder eingeschränkt. Es ist aber so, dass nur die Felder gezogen werden, die in der Zielstruktur vorliegen. Darüber hinaus werden natürlich auch noch jene Felder angeschaut, die Teil der ON- und WHERE-Bedingungen sind, aber nicht Teil der Zielstruktur.

Test dazu: https://community.sap.com/t5/applicatio ... p/13216299

Re: Umherspringende Methoden im quelltextbasierten Class Builder

tar hat geschrieben: ↑

19.10.2024 01:48

Ich nutze zwar kein Eclipse, aber das sieht sehr gut aus - bis auf eine Sache, die ich auch hier immer mal wieder sehe und nicht nachvollziehen kann:

Code: Alles auswählen.

... value #( line_id        = sy-tabix
             typerino       = <component>-type
             element_length = <component>-length
             display_name   = <component>-name
             is_something   = abap_true ) ...

Stattdessen ist diese Einrückung sauberer und die alphabetische Sortierung lesbarer:

Code: Alles auswählen.

... value #(
  element_length = <component>-length
  display_name   = <component>-name
  is_something   = abap_true
  line_id        = sy-tabix
  typerino       = <component>-type
) ...

Da bin ich ganz deutlich anderer Meinung. Das VALUE alleine in eine Zeile zu schreiben und dann in der nächsten Zeile weiterzumachen, ist der gleiche Mist, den der Pretty Printer z.B. mit Funktionsbausteinen macht (wo dann die Schlüsselwörter EXPORTING etc. alleine in einer Zeile stehen). Das wirkt unmotiviert und zieht den Code in die Länge (es passt weniger ins Fenster) und zugleich wird es schlechter lesbar. Ich finde die erste Version in Deinem Beispiel erheblich besser lesbar, pflichte in diesem Fall also Clean ABAP voll bei.

Über alphabetische Reihenfolge kann man streiten. Mir gefällt es besser, mich an die Reihenfolge aus der Strukturdefinition zu halten, allein schon deshalb, weil man dann bei solch VALUE besser erkennen kann, welche Spalten ggf. weggelassen worden sind. Außerdem ist es guter Stil, Primärschlüsselspalten an den Anfang der Struktur zu setzen (bei Datenbanktabellen und den daraus gebildeten Strukturen sind sie ja immer dort). Wenn man das alphabetisch umsortiert, dann sind die Primärschlüsselspalten wild über die Struktur verteilt. Halte ich für fürchterlich. Das lässt mich argwöhnen, dass Du zu der Fraktion gehörst, die alle internen Tabellen als Standardtabellen definieren und sich insofern über Primärschlüssel keine Gedanken machen. In diesem Falle würde ich Dir dringendst nahelegen, Deine diesbezügliche Haltung zu überdenken, da ich interne Tabellen ohne Schlüssel, auf die man suchend zugreift (also die allermeisten), für mittelalterlich und ganz schlechten Stil halte.

Rob_abc hat geschrieben:Ich habe sie einfach untereinander geschrieben. Verstehe nicht, was das weitere Einrücken bringen soll.

Dem stimme ich zu, da INNER JOINs letztlich alle gleichrangig sind, so dass die weiteren Einrückungen eine Hierarchie suggerieren, die es überhaupt nicht gibt.

tar hat geschrieben:SELECTS gestalte ich grundsätzlich so:

Beim SELECT-Befehl gibt es in meinen Augen viele gut lesbare Herangehensweisen, die ich alle für legitim halte. Da ist es nur wichtig, dass sich der Autor überhaupt um eine erkennbare Formatierung bemüht hat. Was mir bei Deiner Version auffällt, ist etwas, was ich auch mache, aber sonst niemals bei irgend jemandem sehe und daher als gutes Indiz nutzen kann, auf einen Blick zu erkennen, welcher Code von mir ist und welcher nicht: INTO vor FROM. Nach meinem Dafürhalten war das einst bei SQL sogar so vorgeschrieben, jedenfalls bei Informix 4GL war diese Reihenfolge Pflicht. In ABAP schreibt aber sonst jeder zuerst FROM und danach erst INTO. ABAP akzeptiert beides.

tar hat geschrieben:INTO CORRESPONDING beeinträchtigt die Performance negativ bei:
- gepufferten Tabellen (hier sollte man einfach SELECT * nutzen)

Tatsächlich halte ich nicht viel von gepufferten Datenbanktabellen. Bei einzelnen Zugriffen ist die Performance egal, und wenn ich oft auf die Tabelle zugreifen muss, dann puffere ich mir die relevanten Spalten in eine interne Tabelle (je nach Situation SORTED oder HASHED) mit passendem Schlüssel. Darauf zuzugreifen ist nochmal viel performanter als jeder Zugriff auf gepuffterte Datenbanktabellen (und als Bonbon kann ich den Suchschlüssel genau so definieren, wie ich ihn für mein Programm optimalerweise brauche).

tar hat geschrieben:- vielen Selektionsfeldern bei nicht-gepufferten Tabellen, die es auch in der Zielstruktur gibt (hier sollte man die konkreten Felder angeben, entweder im SELECT oder in der Typisierung der Zielstruktur oder beides)

Am besten beides und dann nicht INTO CORRESPONDING FIELDS OF TABLE, sondern einfach INTO TABLE. Das ist performanter. Macht heute freilich nicht mehr nennenswert was aus, aber in Zeiten früherer Releases (und der damit einhergehenden alten Hardware) hat das einen richtig wahrnehmbaren Unterschied gemacht.

Re: Umherspringende Methoden im quelltextbasierten Class Builder

Das wird der Grund sein, weshalb die Masse der Entwickler das so macht. Ich habe auch nicht gesagt, dass es verwerflich ist, sondern nur, dass es nach meiner Erinnerung in den Anfängen von SQL andersrum vorgeschrieben gewesen ist.

Es lassen sich aber auch Argumente finden, zuerst INTO und dann FROM zu schreiben. Dann stehen nämlich FROM und WHERE zusammen, was Sinn ergibt, weil man damit sagt: "Selektiere die Zeilen aus der Tabelle xyz, die folgende Eigenschaften erfüllen". Nach meinem Empfinden liest sich das schöner, als wenn Du sagst: "Selektiere die Zeilen aus der Tabelle xyz und schiebe sie nach abc, wenn sie folgende Eigenschaften erfüllen"

Für mich bildet also INTO das Ziel und FROM zusammen mit dem WHERE die Quelle. Wenn man es so sieht, dann ist es schief, das INTO zwischen das FROM und das WHERE zu quetschen.

Aber wie gesagt, ich halte beide Standpunkte für vertretbar.

Re: Umherspringende Methoden im quelltextbasierten Class Builder

DeathAndPain hat geschrieben: ↑

20.10.2024 18:02

Das VALUE alleine in eine Zeile zu schreiben und dann in der nächsten Zeile weiterzumachen, ist der gleiche Mist, den der Pretty Printer z.B. mit Funktionsbausteinen macht (wo dann die Schlüsselwörter EXPORTING etc. alleine in einer Zeile stehen). Das wirkt unmotiviert und zieht den Code in die Länge (es passt weniger ins Fenster) und zugleich wird es schlechter lesbar. Ich finde die erste Version in Deinem Beispiel erheblich besser lesbar, pflichte in diesem Fall also Clean ABAP voll bei.

Das hat mit einer konsistenten Formatierungsausrichtung zu tun, die in vielen gängigen Programmiersprachen so festgelegt ist, dass Unterbestandteile grundsätzlich einen Tabstopp eingerückt werden und vor allem, dass deren Abschluss wieder am Zeilenbeginn der übergeordneten Struktur endet. Beispiel:
Das sieht natürlich genau dann hässlich (und nicht nachvollziehbar) aus, wenn das Untergeordnete aus lediglich einem Bestandteil (1 Zeile) besteht, weswegen man das dann besser in einer Zeile belassen kann, was - wo ich dir zustimme - der Pretty Printer leider nicht berücksichtigt, wobei es dann aber wieder doof aussähe, wenn es bspw. bei Importing mit 1 Parameter direkt dahinter und zusätzlich bei Exporting mit 3 Parametern unten drunter stünde.

DeathAndPain hat geschrieben: ↑

20.10.2024 18:02

Über alphabetische Reihenfolge kann man streiten. Mir gefällt es besser, mich an die Reihenfolge aus der Strukturdefinition zu halten, allein schon deshalb, weil man dann bei solch VALUE besser erkennen kann, welche Spalten ggf. weggelassen worden sind. Außerdem ist es guter Stil, Primärschlüsselspalten an den Anfang der Struktur zu setzen (bei Datenbanktabellen und den daraus gebildeten Strukturen sind sie ja immer dort). Wenn man das alphabetisch umsortiert, dann sind die Primärschlüsselspalten wild über die Struktur verteilt. Halte ich für fürchterlich.

Du kannst das doch halten, wie du magst. Du siehst aber generell nicht per se, welche Felder Teil des Primärschlüssels sind - immer erst nach dem Absprung in die DB-Tabelle und da finde ich die meist nicht sprechenden Spaltennamen fürchterlich und möchte nicht ständig hin- und herspringen müssen, um zu wissen, was sie bedeuten, sondern orientiere mich an der verständlicheren Benamung der Zielstruktur, die ich selbst vornehmen kann, danach sortiere und dadurch sofort sehe, welche inhaltlichen Informationen genutzt werden. Die Behandlung der Primärschlüssel findet sich indes im WHERE und ON, wobei ich das so wie du mache, obwohl die dortige Sortierung der DB selbst eigentlich egal ist.

Es ist ohnehin sinnvoll, die Schlüssel ordentlicher zu benennen, bspw. bei klaren Entitäten eben einfach "ID", da es vollkommener Irrsinn wäre, jedesmal erst die Struktur anschauen zu müssen, um herauszufinden, dass bspw. ein Zählpunkt INT_UI und ein Gerät EQUNR als Schlüssel nutzt. Dahingehend sähe es dann bei der Typisierung schön doof aus, wenn ID (oder INT_UI) ganz oben und danach alphetisch sortiert würde oder grundsätzlich alles unsortiert aussieht, weil die Entitätsdaten aus 6 verschiedenen Tabellen gezogen werden, nach deren Sortierung du dich beim Typisieren orientierst, während sie sich in der strukturellen Sortierung bei jedem SPAU-Lauf ändern könnten. Von Kundentabellen ganz zu schweigen 😉

DeathAndPain hat geschrieben: ↑

20.10.2024 18:02

INTO vor FROM. Nach meinem Dafürhalten war das einst bei SQL sogar so vorgeschrieben, jedenfalls bei Informix 4GL war diese Reihenfolge Pflicht. In ABAP schreibt aber sonst jeder zuerst FROM und danach erst INTO. ABAP akzeptiert beides.

Ein weiterer Grund ist die optische Aufbereitung:
Die doppelten Leerzeichen zur vertikalen Ausrichtung beim INTO sehen hier einfach nicht mehr schön aus während sie über dem FROM optisch passen. Manchmal muss man aber das INTO leider am Ende schreiben - weiß grad nicht, ob es bei GROUP BY oder anderen speziellen SQL-Befehlen war.

In anderen Sprachen gibt es gar kein INTO, d.h. das INTO ist quasi der Aufrufer.

Re: Umherspringende Methoden im quelltextbasierten Class Builder

tar hat geschrieben: ↑

21.10.2024 01:30

Das hat mit einer konsistenten Formatierungsausrichtung zu tun, die in vielen gängigen Programmiersprachen so festgelegt ist, dass Unterbestandteile grundsätzlich einen Tabstopp eingerückt werden und vor allem, dass deren Abschluss wieder am Zeilenbeginn der übergeordneten Struktur endet. Beispiel:

Code: Alles auswählen.

data(lo_person) = new lcl_person(
  age    = 43
  gender = c_male
  name   = value #(
    first = |Archibald|
    last  = |Tuttle|
  )
).

Ganz ehrlich: Dein obenstehendes Beispiel finde ich optisch scheußlich. Das fängt schon damit an, dass "age" in einer neuen Zeile steht und damit optisch komplett vom Methodenaufruf losgelöst ist (während zugleich der Block eine Zeile länger wird).

Ob man die schließende Klammer in eine eigene Zeile stellt (und das Konstrukt damit wieder um eine Zeile aufbläht), kann man sich streiten. Das Argument dafür, das ich sehe, besteht darin, dass man so dem VALUE leicht Spalten hinzufügen kann, ohne sich jedesmal die Frage stellen zu müssen, ob es die letzte Spalte ist und man daher die Klammer am Ende dahinterpacken muss. Das Argument dagegen besteht darin, dass man so (wie in Deinem Beispiel gleich zweimal zu sehen) eine häßliche Klammer einsam in einer Zeile bekommt, was sich gleichzeitig schlecht liest und das Konstrukt eine Zeile länger macht. Wohlgemerkt: ich bin nicht dafür, möglichst viel in eine Zeile zu packen und dafür Spaghetticode zu akzeptieren. Aber wenn durch das Verteilen auf zusätzliche Zeilen die Lesbarkeit sogar schlechter wird, dann ist das für mich keine gute Entscheidung.

Die von Dir formulierte Formatierungsregel habe ich zwar verstanden, aber am Ende geht es mir nicht darum, sklavisch irgendwelche akademischen Regeln zu befolgen, sondern ein Format zu wählen, das sich übersichtlich und ansprechend liest. Ich hätte es so geschrieben:
Und da behaupte ich, dass das nicht nur kürzer (weniger Zeilen) ist, sondern sich zugleich auch besser liest. Und wenn das so ist, dann werde ich es nicht anders machen, um irgendwelche Formalien zu genügen, die mutmaßlich von Konstrukten wie Schleifen oder IF-Blöcken stammen. Dass man zwischen LOOP und ENDLOOP oder IF und ENDIF einrückt, da bin ich voll dafür. Das liest sich wunderbar. Aber das ist doch kein Grund, dieses Prinzip zwanghaft auch auf Situationen anzuwenden, bei denen es die Lesbarkeit tatsächlich sogar verschlechtert.

tar hat geschrieben:was - wo ich dir zustimme - der Pretty Printer leider nicht berücksichtigt, wobei es dann aber wieder doof aussähe, wenn es bspw. bei Importing mit 1 Parameter direkt dahinter und zusätzlich bei Exporting mit 3 Parametern unten drunter stünde.

Der macht es ja noch schlimmer: der packt den Methodenaufruf in eine Zeile, dann das Schlüsselwort EXPORTING in eine weitere Zeile und dann den nächsten Parameter in wieder eine weitere Zeile. Das ist komplett unnötig und liest sich miserabel. Aufruf, EXPORTING-Schlüsselwort und der erste Parameter können problemlos in einer Zeile stehen und etwaige weitere Parameter dann bündig unter dem ersten. Übersichtlicher geht es nicht, und zugleich braucht man weniger Zeilen dafür und hat so mehr auf dem Bildschirm.

tar hat geschrieben:Du kannst das doch halten, wie du magst. Du siehst aber generell nicht per se, welche Felder Teil des Primärschlüssels sind - immer erst nach dem Absprung in die DB-Tabelle

Falsch. Den Schlüssel Deiner internen Tabelle siehst Du nicht in der Datenbanktabelle, sondern in der Typdefinition Deiner internen Tabelle!. Selbst wenn Du faul bist und Deine interne Tabelle einfach über die DB-Tabelle definierst, statt Dir die Mühe zu machen, in der Typdefinition diejenigen Spalten aufzuzählen, die Du in Deiner Routine tatsächlich brauchst (das sind fast nie alle), kannst Du immer noch schreiben:
Ob der Schlüssel, den Du dabei wählst, identisch zum DB-Tabellenschlüssel ist oder anders, ist dabei Dir überlassen, und Du kannst es auf die Bedürfnisse Deines Programms feintunen. Das kann sogar bedeuten, dass Du einen Schlüssel wählst, der nicht UNIQUE ist. Einziges Kriterium ist, wie Dein Programm im weiteren Verlauf auf die interne Tabelle zuzugreifen gedenkt. Solltest Du allerdings einen Schlüssel wählen, der aus Deiner Sicht UNIQUE sein müsste, dann solltest Du ihn auch so definieren. Ich weiß nicht, wie viele Male mich nützliche ABAP-Dumps auf Konsistenzfehler in meinen Programmen aufmerksam gemacht haben, die ich andernfalls nicht bemerkt hätte! Aus demselben Grunde benutze ich auch gerne den ASSERT-Befehl, den viele nicht mal kennen. Wenn ich z.B. eine Zeile meiner internen Tabelle lese und mir sicher bin, dass es diese Zeile geben muss, dann schreibe ich gern mal:
Damit spare ich mir eine explizite Behandlung des Falls, dass die Zeile nicht gefunden wird, weil ich ja aufgrund meiner Programmstruktur genau weiß, dass es sie geben muss. Zugleich sorge ich damit aber auch dafür, dass mein Programm spektakulär auf die Nase fällt, wenn ich einen Denkfehler gemacht habe und es die Zeile doch mal nicht gibt. Dann ist ein Dump genau richtig, denn wenn das Programm sich in einer Weise verhält, bei der ich als Programmierer überzeugt bin, dass das gar nicht passieren kann, dann habe ich einen Denkfehler gemacht und sein Verhalten ist undefiniert. Alles schon vorgekommen, und dann ist das ein Bug, den ich finden und beheben muss. Lieber lasse ich auch einen Anwender mit solch Dump auf die Bretter gehen, als dass ihm das Programm auf undefinierte Weise weiterläuft und damit fast zwingend falsche Ergebnisse bringt.

Nebenbei zeige ich auch jedem Leser meines Codes damit klar, dass die Existenz dieser Tabellenzeile garantiert ist.

tar hat geschrieben:Es ist ohnehin sinnvoll, die Schlüssel ordentlicher zu benennen, bspw. bei klaren Entitäten eben einfach "ID", da es vollkommener Irrsinn wäre, jedesmal erst die Struktur anschauen zu müssen

Das sehe ich ganz deutlich anders. Was Du da machen willst, ist eine Erweiterung der eh schon überflüssigen ungarischen Notation. Interne Tabellen sollten klar verständlich typisiert sein. Den Schlüssel an den Anfang der Tabelle zu stellen ist dabei eine gute Idee, und inder Mitte der Tabelle versteckte "ID"-Spalten eine gruselige Alternative (auch wenn Du so erreichst, dass sie alle hintereinander stehen, weil sie alle mit ID anfangen. Das heißt immer noch nicht, dass sie dann in der richtigen Reihenfolge (wie im Schlüssel) hintereinander stehen, weil Du ja - aus meiner Sicht komplett unmotiviert - alphabetisch sortieren möchtest. Also müsstest Du sie ID1..., ID2... usw. nennen. Sorry, aber was für ein Krampf!