Seite 1 von 1
Antipattern
Verfasst: 11. April 2018 18:00
von Nody3000
Hallo zusammen,
ich sehe oft das eine oder andere Antipattern in Dynamics Nav und möchte vorschlagen das wir ähnlich wie Tips und Ticks halt auch mal eintragen was wir sehen oder eben nicht sehen wollen im Code.
Gerne mit Feedback wann ein
Anipattern zur
technischen Schuld wird oder eben nicht.
Häufig genug sind schon die einfachsten Regeln im
Roten Grad von Clean Code Development verletzt.
Es ist mir schon oft vorgekommen das Leute die Object Manager Advanced einsetzen denken das sie damit eine Versionskontrolle hätten oder z.b.
diese kleine Anekdote (Isempty <> Findfirst) die ich aus einem Video von Marije Brummel erfahren habe. IsEmpty muss wohl echt für viel Support gesorgt haben.
Gruß Nody3000
Re: Antipattern
Verfasst: 12. April 2018 16:20
von Timo Lässer
"Ich habe keine Zeit meine Axt zu schärfen, denn ich muss Bäume fällen!"Problem: Anstatt sich im Vorfeld detailliert Gedanken über Einheitlichkeit, Modularität, Erweiterbarkeit, ... zu machen, wird erstmal wild drauf los programmiert.
Konsequenz: Spaghetti-Code, Doppelter Code, ...
"Das haben wir schon immer so gemacht."Problem: Nur weil etwas früher gut war, heißt es noch lange nicht, dass es heute keine besseren Möglichkeiten gibt.
Konsequenz: Neue Technologien können nicht genutzt werden und/oder neue Module im Standard bleiben ungenutzt, weil man früher dafür etwas individuelles verwendet hatte.
- Doppelte Verneinungen
- Code:
IF NOT Cust."No Discount allowed" THEN
[...]
- Boolean-Felder mit = TRUE oder =FALSE abfragen
- Code:
IF Item.Blocked = TRUE THEN
[...]
- FINDFIRST in einer Schleife verwenden
- Code:
IF Cust.FINDFIRST THEN
REPEAT
[...]
UNTIL Cust.NEXT = 0;
- Mehrere Anweisungen in einer einzigen Zeile
- Code:
IF [...] THEN [...] ELSE IF [...] THEN [...] ELSE [...];
Re: Antipattern
Verfasst: 12. April 2018 16:27
von Natalie
Das NAV-Team bei Microsoft hat mal hier welche gesammelt:
Reusable Bugs
Re: Antipattern
Verfasst: 12. April 2018 20:08
von Nody3000
Wow, ihr legt ja hier direkt los.
Re: Antipattern
Verfasst: 13. April 2018 10:31
von Danis
Natalie hat geschrieben:Das NAV-Team bei Microsoft hat mal hier welche gesammelt:
Reusable Bugs
Danke, dass hatte ich im Hinterkopf aber nicht mehr gefunden.
Re: Antipattern
Verfasst: 13. April 2018 10:41
von Natalie
Re: Antipattern
Verfasst: 13. April 2018 18:06
von Nody3000
Ja, genau sowas.
WITH DO ist für mich generell ein Antipattern. Ich benutze es seit Jahren schon nicht mehr.
https://community.dynamics.com/nav/w/de ... -collision- Code:
With Rec Do
10x Preusocodelines;
With OtherRec DO
Function("No.");
Das generelle resetten von Variablen noch vor der ersten Benutzung ist für mich auch ein Antipattern.
Es lässt mich zweifeln ob die Variable lokal ist und denken das der andere Programmierer evtl. nicht weiß wie lokale funktionieren.
Zusätzlich ist es eine unnötige Zeile sofern das Clear nicht gezielt etwas bewirken soll.
Generell kann man Reset und Clear vermutlich oft schon vermeiden wenn man "lokal" programmiert.
- Code:
PROCEDURE Function@50000(Parameter) : Bool;
VAR
Record@50000 : Record 50000;
BEGIN
CLEAR(Record);
Rec.SetFilter("No.",'123');
Exit(Rec.IsEmpty);
END;
Re: Antipattern
Verfasst: 16. April 2018 09:18
von shove
Nody3000 hat geschrieben:WITH DO ist für mich generell ein Antipattern.
Dem kann ich nicht ganz zustimmen
Ich denke das WITH DO hat seine Daseinsberechtigung. Vor allem dann, wenn ich z.B. so was mache:
- Code:
LOCAL BeispielFunktion()
WITH Contact DO
IF FINDSET THEN
REPEAT
//....
UNTIL NEXT = 0;
Natürlich ist, wie du schon sagtest, das Verschachteln von WITH DO keine so tolle Idee, und zählt für mich damit auch zum Antipattern.
Re: Antipattern
Verfasst: 16. April 2018 10:30
von Timo Lässer
Natürlich ist es praktisch, seinen Programmcode mittels WITH zu kürzen, jedoch birgt es auch seine heimtückischen Gefahren:
Neue Felder können Standard ohne Codeänderung verändern
Re: Antipattern
Verfasst: 16. April 2018 10:41
von fiddi
Hallo,
auch die Verwendung von lokalen und globalen Variablen mit gleichem Namen kann zu ganz merkwürdigen Effekten führen.
Gruß Fiddi
Re: Antipattern
Verfasst: 16. April 2018 11:18
von shove
Timo Lässer hat geschrieben:Natürlich ist es praktisch, seinen Programmcode mittels WITH zu kürzen, jedoch birgt es auch seine heimtückischen Gefahren
Stimmt darüber habe ich bisher auch noch nicht nachgedacht.
Mit dieser Grundlage erscheint mir dann auch dieser Artikel sinnvoller als bisher:
https://docs.microsoft.com/de-de/dynami ... fix-suffixIm Text ist beschrieben, dass es nicht nur Sinn macht Objekte sondern auch Felder und Controls zu Pre-/Suffixen. (Sofern Table/Page-Extension)
Re: Antipattern
Verfasst: 18. April 2018 14:18
von Nody3000
Genau sowas ist mir heute passiert. Und trifft auf das von fiddi beschriebene Problem gleichermaßen zu.
Die Funktion ist im Excelbuffer.
- Code:
EnterSum(ColNo : Integer;RowNo : Integer;FromRowNo : Integer;FromColNo : Integer;ToRowNo : Integer;ToColNo : Integer;Bold : Boolean;Underline : Boolean;NumberFormat : Text[50])
INIT;
VALIDATE("Row No.",RowNo);
VALIDATE("Column No.",ColNo);
"Cell Value as Text" := '';
Formular := 'XY';
Bold := Bold; <- Hier wurde der Variable selbst der eigene Wert zugewiesen.
Underline := Underline; <- Hier wurde der Variable selbst der eigene Wert zugewiesen.
NumberFormat := NumberFormat; <- Hier wurde der Variable selbst der eigene Wert zugewiesen.
INSERT;
Ich habe es wie folgt abgeändert.
- Code:
EnterSum(ColNo : Integer;RowNo : Integer;FromRowNo : Integer;FromColNo : Integer;ToRowNo : Integer;ToColNo : Integer;P_Bold : Boolean;P_Underline : Boolean;P_NumberFormat : Text[50])
INIT;
VALIDATE("Row No.",RowNo);
VALIDATE("Column No.",ColNo);
"Cell Value as Text" := '';
Formular := 'XY';
Bold := P_Bold;
Underline := P_Underline;
NumberFormat := P_NumberFormat;
INSERT;
Zeitgleich kann man damit meines Erachtens Parameter Namenskollisionen vermeiden als auch G_Variable <> L_Variable verhalten vermeiden. Auch hat sich das eindeutige Prefix mit Variablenname bei Textersetzungen bewährt.
Auch der Parameter Validate("Row No.",RowNo) ist meiner Meinung nach zu ähnlich. Funktioniert so aber erstmal. Ich würde es refactoren wenn es die Naming-Regeln zulassen würden.
@shove den Link werde ich mal intern Vorschlagen ob unsere Naming Regeln nicht evtl. verbessert werden können. Danke
Re: Antipattern
Verfasst: 18. April 2018 17:58
von fiddi
Hallo,
wo du gerade beim Excel- Buffer bist:
Wer sich schon immer gewundert hat, warum der Excel-Buffer beim Import immer so schnell auf 100 % ist, es aber doch oft noch lange dauert bis er fertig ist, sollte sich mal den folgenden Code anschauen:
- Code:
Enumerator := XlWrkShtReader.GetEnumerator;
RowCount := XlWrkShtReader.RowCount;
WHILE Enumerator.MoveNext DO BEGIN
CellData := Enumerator.Current;
IF CellData.HasValue THEN BEGIN
VALIDATE("Row No.",CellData.RowNumber);
VALIDATE("Column No.",CellData.ColumnNumber);
ParseCellValue(CellData.Value,CellData.Format);
INSERT;
END;
i := i + 1;
COMMIT;
IF NOT UpdateProgressDialog(ExcelBufferDialogMgt,LastUpdate,i,RowCount) THEN BEGIN
QuitExcel;
ERROR(Text035)
END;
END;
Was läuft hier falsch?
Hier sind gleich mehrere Pattern enthalten:
- Variablen sollten sinnvolle Namen haben.
- Man sollte sich immer im Klaren sein, welche Größen man kombiniert/vergleicht.
- Nach Möglichkeit sollte man COMMITs im Quelltext vermeiden.
Gruß Fiddi
Re: Antipattern
Verfasst: 18. April 2018 18:56
von fiddi
So etwas gehört für mich auch zu den Antipattern:
- Code:
IF SalespersonPurchaser.GET(GenJournalLine2."Salespers./Purch. Code") THEN
IF SalespersonPurchaser.VerifySalesPersonPurchaserPrivacyBlocked(SalespersonPurchaser) THEN
ERROR(SalespersonPurchPrivacyBlockErr,GenJournalLine2."Salespers./Purch. Code");
Der Funktion "VerifySalesPersonPurchaserPrivacyBlocked" noch mal den Record zu übergeben, obwohl klar ist, das man diese Funktion immer nur mit dem aktuellen Salesperson- Record aufrufen wird.
Besser fände ich den folgenden Code. Die aufgerufene Funktion wird auch gleich viel lesbarer.
- Code:
IF SalespersonPurchaser.GET(GenJournalLine2."Salespers./Purch. Code") THEN
IF SalespersonPurchaser.VerifySalesPersonPurchaserPrivacyBlocked THEN
ERROR(SalespersonPurchPrivacyBlockErr,GenJournalLine2."Salespers./Purch. Code");
Außerdem werden Fehler vermieden, die entstehen wenn bei der Programmierung der Funktion doch mal eine Variable aus Rec und übergebenem Record verwechselt werden.
- Code:
VerifySalesPersonPurchaserPrivacyBlocked(SalespersonPurchaser2 : Record "Salesperson/Purchaser") : Boolean
BEGIN
IF "Privacy Blocked" THEN
EXIT(TRUE);
EXIT(FALSE);
END;
Dieser Code ließe sich problemlos compilieren, würde in 99% aller Fälle auch funktionieren, weil REC gleich SalespersonPurchaser2 ist. Nur in dem einen Fall, beim dem REC nicht gleich SalespersonPurchaser2 ist, geht es schief. Diesen Fehler kannst du u.U. Monatelang suchen.
Gruß Fiddi
Re: Antipattern
Verfasst: 31. August 2018 12:19
von Nody3000
Zum Records übergeben generell,
ich sehe mir gerade wieder Code an in dem ganze Records übergeben werden und am Ende nur eine Nummer benötigt wird. Sagen wir mal die Kontaktnummer.
Ich habe teilweise sogar gesehen das die Funktion gedoppelt wurden (DRY Prinzip verletzt) damit man sie von Tabelle A und Tabelle B mit "Rec" aufrufen kann statt den Aufruf mal zu verallgemeinern.
Ich übergebe nur Records wenn das Szenario sich nur so lösen lässt z.B. ein Vergleich von Rec und XRec oder ich mehr als 5 Parameter definieren müsste, ich also durchaus mehrere Felder vom Record brauche.
Wie ist eure Guideline dort so ?
Re: Antipattern
Verfasst: 31. August 2018 12:55
von Timo Lässer
Wenn ich ein und dieselbe Funktion für mehrere Tabellen (z. B. Sales Header, Purchase Header, Transfer Header, Service Header, ...) benötige, dann setze ich von Anfang an auf RecordRef.
So kann ich diese Funktion später auch problemlos mit einem Production Order oder Assembly Header aufrufen, ohne die Funktion anpassen zu müssen.
Re: Antipattern
Verfasst: 31. August 2018 13:00
von Kowa
Nody3000 hat geschrieben:ich sehe mir gerade wieder Code an in dem ganze Records übergeben werden
Solange die als Referenz (also mit var) übergeben werden ist das kein Problem und flexibler, falls doch irgendwann weitere Felder benötigt werden. Unperformant wird es nur, wenn man tatsächlich den ganzen Datensatz (ohne var) durch die Funktionen schaufelt.
Re: Antipattern
Verfasst: 13. November 2018 16:02
von ERP-Berater
Frage: könnte es sein, dass in der CU 13, folgender Code auch ein Antipattern ist?
- Code:
IF STRLEN(INCSTR(GenJnlBatch.Name)) > MAXSTRLEN(GenJnlBatch.Name) THEN
IMHO ist INCSTR unnötig. Es braucht nur die Abfrage, ob Feld Name einen Text > 10 Zeichen enthält und damit die Datentypbegrenzung Text Länge 10 sprengt. Interessanterweise benützt die CU 13 später mehrmals INCSTR auf Buchblattname. Der Sinn dahinter erschliesst sich nicht. Es sieht so aus, als hätte vor 10-20 Jahren ein Entwickler Buchblattname für eine Art Nummernserie gehalten.
Re: Antipattern
Verfasst: 13. November 2018 16:13
von Natalie
Der Quelltext fragt ab, ob es unmöglich ist, die Nummer weiter zu erhöhen, ohne dass die Maximallänge überschritten würde.
Warum? Weil (je nach Einrichtung) nach jeder Buchung automatisch ein neuer Buchblattname angelegt wird. Hieß er vorher MEINBB001, dann heißt der neue: MEINBB002, eben weil bei der Erzeugung sehr wohl mit INCSTR gearbeitet wird. Und wenn es vorher MEINBB999 hieß, dann hätten wir nach der Buchung tatsächlich ein Zeichen Zuviel.
Re: Antipattern
Verfasst: 13. November 2018 16:32
von ERP-Berater
Natalie hat geschrieben:Der Quelltext fragt ab, ob es unmöglich ist, die Nummer weiter zu erhöhen, ohne dass die Maximallänge überschritten würde.
Warum? Weil (je nach Einrichtung) nach jeder Buchung automatisch ein neuer Buchblattname angelegt wird. Hieß er vorher MEINBB001, dann heißt der neue: MEINBB002, eben weil bei der Erzeugung sehr wohl mit INCSTR gearbeitet wird. Und wenn es vorher MEINBB999 hieß, dann hätten wir nach der Buchung tatsächlich ein Zeichen Zuviel.
Ja, ABER: wenn der Buchblattname KEINE Zahl enthält, wird kein neuer angelegt.
Das mit der Neuanlage hab ich auch schon rausbekommen.
Ich bezweifle aber, dass dies betriebswirtschaftlich einen Sinn hat.
In der MOC Schulungsunterlage sind die Buchblattnamen mit Meier, Müller, Berger benannt.
Ich hab eher den Eindruck, dass der Programmierer bei MS vor 10 oder 20 Jahren in NAV einfach nicht gewusst hat, wofür Buchblattname verwendet wird.
Nämlich zur betriebswirtschaftlich-organisatorischen Abrenzung von zB mehreren Buchhaltern, Kostenrechner etc.
Es macht IMHO wenig Sinn, STANDARD, dann STANDARD_01 .. STANDARD_09 zu haben.
Re: Antipattern
Verfasst: 13. November 2018 16:43
von fiddi
Hallo,
es gibt aber auch Leute, die benutzen das mit Absicht, um zu sehen, ob Sie bestimmte Dinge schon erledigt haben.
Ich glaube nicht, das sich die Programmierer in NAV Ur- Zeiten nicht etwas bei dieser Konstruktion gedacht haben. Die haben mehr über Dinge nachgedacht, als als die meisten heutigen Programmierer.
z.B: das Buchblatt: "MIETE2018/01" Wenn man es gebucht hat, weiß man, das man es in dem jeweiligen Monat erledigt hat.
Gruß Fiddi
Re: Antipattern
Verfasst: 13. November 2018 16:48
von ERP-Berater
fiddi hat geschrieben:Hallo,
z.B: das Buchblatt: "MIETE2018/01" Wenn man es gebucht hat, weiß man, das man es in dem jeweiligen Monat erledigt hat.
Gruß Fiddi
Einspruch! Das ist ein wiederkehrender Geschäftsprozess. Hier macht INCSTR sehr wohl Sinn. Aber bei fire_and_forget Buchungen, die nicht mehr auftreten, sollte es IMHO kein INCSTR geben.
EDIT: und bei Textlänge max 10 Zeichen, macht das Bsp. 01/2018 wenig Sinn, da das Jahr ja 12 Monate hat und der Error schon beim Übergang 9 auf 10 kommt.
Re: Antipattern
Verfasst: 13. November 2018 17:00
von fiddi
Hallo,
ebenfalls Einspruch. Du musst ja keine Zahlen in den Buchblattnahmen eintragen, dann passiert ja auch nichts. Weshalb man sich seine Buchblattstrukturen/ -namen schon ein wenig überlegen sollte.
Gruß Fiddi
Re: Antipattern
Verfasst: 13. November 2018 18:05
von fiddi
Ich hab noch die uralten Versionen am Anfang meiner Laufbahn erlebt und erinnere mich, dass beim Übergang auf 5.00 einige Text Feldlängen von 30 auf 10 gekürzt wurden. Leider hab ich mir nicht gemerkt, welche Felder von welchen Tabellen das waren.
Den Buchblattnamen haben sie jedenfalls nicht gekürzt, der war auch in NAV2.01 schon 10 Zeichen lang.
Gruß Fiddi
Re: Antipattern
Verfasst: 15. November 2022 11:08
von Kowa
Nody3000 hat geschrieben:WITH DO ist für mich generell ein Antipattern. Ich benutze es seit Jahren schon nicht mehr.
Ich auch nicht, aber andere schon
. Dazu gibt es ab BC 21 Änderungen, nachdem es schon seit BC 17 auf
Deprecated stand:
Deprecating Explicit and Implicit With StatementsWith Business Central 2022 release wave 2, the AL:Go! template for creating new AL projects in Visual Studio Code, now enables explicit with statements by default, by adding the NoImplicitWith option to the features property in the generated app.json file.
Also ein neuer Schalter als Vorgabe
NoImplicitWith in der app.json, um ein Explicit
With weiterhin nutzen zu können.