formatted nav patterns 2 to 4
This commit is contained in:
parent
2fad891a77
commit
f1f4aab1d7
55 changed files with 788 additions and 506 deletions
|
|
@ -8,18 +8,18 @@ Unintentional value changes might propagate. Also, it might lead people to belie
|
|||
|
||||
Bad code
|
||||
|
||||
LOCAL PROCEDURE ShowMessage@15(VAR Text@1000 : Text\[250\]);
|
||||
LOCAL PROCEDURE ShowMessage@15(VAR Text@1000 : Text[250]);
|
||||
BEGIN
|
||||
Text := GetMessageText;
|
||||
IF (Text <\> '') AND GenJnlLineInserted THEN
|
||||
MESSAGE(Text);
|
||||
Text := GetMessageText;
|
||||
IF (Text <> '') AND GenJnlLineInserted THEN
|
||||
MESSAGE(Text);
|
||||
END;
|
||||
|
||||
Good code
|
||||
|
||||
LOCAL PROCEDURE ShowMessage@15(Text@1000 : Text\[250\]);
|
||||
LOCAL PROCEDURE ShowMessage@15(Text@1000 : Text[250]);
|
||||
BEGIN
|
||||
Text := GetMessageText;
|
||||
IF (Text <\> '') AND GenJnlLineInserted THEN
|
||||
MESSAGE(Text);
|
||||
Text := GetMessageText;
|
||||
IF (Text <> '') AND GenJnlLineInserted THEN
|
||||
MESSAGE(Text);
|
||||
END;
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ Do not write functions that have high class coupling. This makes the code hard t
|
|||
|
||||
Bad code
|
||||
|
||||
Any procedure / trigger that has class coupling of \> 30
|
||||
Any procedure / trigger that has class coupling of > 30
|
||||
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ Do not write functions that have high cyclomatic complexity. This makes the code
|
|||
|
||||
Bad code
|
||||
|
||||
Any procedure / trigger that has a cyclomatic complexity \> 25, using the CC3 version mentioned in [this article][anchor0].
|
||||
Any procedure / trigger that has a cyclomatic complexity > 25, using the CC3 version mentioned in [this article][anchor0].
|
||||
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -7,23 +7,23 @@ FINDSET, FIND('+') or FIND('-') should only be used when NEXT is used and vice v
|
|||
Bad code
|
||||
|
||||
IF Cust.FIND('-') THEN
|
||||
ERROR(CustIsBlockErr)
|
||||
ERROR(CustIsBlockErr)
|
||||
|
||||
Good code
|
||||
|
||||
IF Cust.FINDFIRST THEN
|
||||
ERROR(CustIsBlockErr)
|
||||
ERROR(CustIsBlockErr)
|
||||
|
||||
Bad code
|
||||
|
||||
IF Cust.FINDFIRST THEN
|
||||
REPEAT
|
||||
...
|
||||
UNTIL Cust.NEXT = 0;
|
||||
REPEAT
|
||||
...
|
||||
UNTIL Cust.NEXT = 0;
|
||||
|
||||
Good code
|
||||
|
||||
IF Cust.FINDSET THEN
|
||||
REPEAT
|
||||
...
|
||||
UNTIL Cust.NEXT = 0;
|
||||
REPEAT
|
||||
...
|
||||
UNTIL Cust.NEXT = 0;
|
||||
|
|
|
|||
|
|
@ -8,25 +8,25 @@ Bad code
|
|||
|
||||
PROCEDURE SetPurchLine@22(VAR CurrentPurchLine@1000 : Record 39);
|
||||
VAR
|
||||
Pegging@1001 : Boolean;
|
||||
Pegging@1001 : Boolean;
|
||||
BEGIN
|
||||
IF Pegging THEN
|
||||
CurrQuantity := CurrentPurchLine."Quantity (Base)"
|
||||
ELSE
|
||||
CurrQuantity := CurrentPurchLine."Outstanding Qty. (Base)";
|
||||
IF Pegging THEN
|
||||
CurrQuantity := CurrentPurchLine."Quantity (Base)"
|
||||
ELSE
|
||||
CurrQuantity := CurrentPurchLine."Outstanding Qty. (Base)";
|
||||
END;
|
||||
|
||||
Good code
|
||||
|
||||
PROCEDURE SetPurchLine@22(VAR CurrentPurchLine@1000 : Record 39);
|
||||
VAR
|
||||
Pegging@1001 : Boolean;
|
||||
Pegging@1001 : Boolean;
|
||||
BEGIN
|
||||
Pegging := IsPegging(CurrentPurchLine);
|
||||
IF Pegging THEN
|
||||
CurrQuantity := CurrentPurchLine."Quantity (Base)"
|
||||
ELSE
|
||||
CurrQuantity := CurrentPurchLine."Outstanding Qty. (Base)";
|
||||
Pegging := IsPegging(CurrentPurchLine);
|
||||
IF Pegging THEN
|
||||
CurrQuantity := CurrentPurchLine."Quantity (Base)"
|
||||
ELSE
|
||||
CurrQuantity := CurrentPurchLine."Outstanding Qty. (Base)";
|
||||
END;
|
||||
|
||||
Bad code
|
||||
|
|
@ -34,16 +34,16 @@ Bad code
|
|||
// In the example below, the function will always return FALSE.
|
||||
PROCEDURE GetItemsToPlan@22() : Boolean;
|
||||
BEGIN
|
||||
SETRANGE("Document Type","Document Type"::Order);
|
||||
...
|
||||
FINDSET
|
||||
SETRANGE("Document Type","Document Type"::Order);
|
||||
...
|
||||
FINDSET
|
||||
END;
|
||||
|
||||
Good code
|
||||
|
||||
PROCEDURE GetItemsToPlan@22() : Boolean;
|
||||
BEGIN
|
||||
SETRANGE("Document Type","Document Type"::Order);
|
||||
...
|
||||
EXIT(FINDSET)
|
||||
SETRANGE("Document Type","Document Type"::Order);
|
||||
...
|
||||
EXIT(FINDSET)
|
||||
END;
|
||||
|
|
|
|||
|
|
@ -12,11 +12,11 @@ Bad code
|
|||
|
||||
Good code
|
||||
|
||||
Any procedure / trigger that has a maintainability index \>= 20\.
|
||||
Any procedure / trigger that has a maintainability index >= 20\.
|
||||
The Maintainability Index is computed as a function:
|
||||
- Lines Of Code (inverse proportional)
|
||||
- the Halstead Volume
|
||||
- Cyclomatic Complexity (inverse proportional).
|
||||
- Lines Of Code (inverse proportional)
|
||||
- the Halstead Volume
|
||||
- Cyclomatic Complexity (inverse proportional).
|
||||
|
||||
More info
|
||||
|
||||
|
|
@ -25,7 +25,7 @@ More info
|
|||
|
||||
Bad code
|
||||
|
||||
Any procedure / trigger that is \> 100 lines of code
|
||||
Any procedure / trigger that is > 100 lines of code
|
||||
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -4,8 +4,6 @@ weight = 920
|
|||
+++
|
||||
The number of parameters passed to a string must match the placeholders.
|
||||
|
||||
****
|
||||
|
||||
Bad code
|
||||
|
||||
CannotDeleteLineErr@1005 : TextConst 'ENU=You cannot delete this line because one or more rating values exists.';
|
||||
|
|
@ -18,7 +16,7 @@ Good code
|
|||
...
|
||||
ERROR(CannotDeleteLineErr);
|
||||
|
||||
###
|
||||
|
||||
|
||||
Bad code
|
||||
|
||||
|
|
|
|||
|
|
@ -8,14 +8,14 @@ Bad code
|
|||
|
||||
LOCAL PROCEDURE Code@1();
|
||||
VAR
|
||||
CAJnlPostBatch@1001 : Codeunit 1103;
|
||||
CAJnlPostBatch@1001 : Codeunit 1103;
|
||||
BEGIN
|
||||
CAJnlPostBatch.Run(CostJnlLine);
|
||||
CAJnlPostBatch.Run(CostJnlLine);
|
||||
END;
|
||||
|
||||
Good code
|
||||
|
||||
LOCAL PROCEDURE Code@1();
|
||||
BEGIN
|
||||
CODEUNIT.RUN(CODEUNIT::"CA Jnl.-Post Batch",CostJnlLine);
|
||||
CODEUNIT.RUN(CODEUNIT::"CA Jnl.-Post Batch",CostJnlLine);
|
||||
END;
|
||||
|
|
|
|||
|
|
@ -8,18 +8,18 @@ It affects code readability and can lead to wrong assumptions.
|
|||
|
||||
Bad code
|
||||
|
||||
IF Type <\> Type::FIELD THEN BEGIN
|
||||
...
|
||||
ERROR(...);
|
||||
RecRef.CLOSE;
|
||||
IF Type <> Type::FIELD THEN BEGIN
|
||||
...
|
||||
ERROR(...);
|
||||
RecRef.CLOSE;
|
||||
END;
|
||||
|
||||
|
||||
|
||||
Good code
|
||||
|
||||
IF Type <\> Type::FIELD THEN BEGIN
|
||||
...
|
||||
RecRef.CLOSE;
|
||||
ERROR(...);
|
||||
IF Type <> Type::FIELD THEN BEGIN
|
||||
...
|
||||
RecRef.CLOSE;
|
||||
ERROR(...);
|
||||
END;
|
||||
|
|
|
|||
|
|
@ -6,33 +6,33 @@ The value assigned to a variable must be used. Else the variable is not necessar
|
|||
|
||||
Bad code
|
||||
|
||||
PROCEDURE AddEntities@1(FilterStr@1000 : Text\[250\]);
|
||||
PROCEDURE AddEntities@1(FilterStr@1000 : Text[250]);
|
||||
VAR
|
||||
Vendor@1001 : Record 23;
|
||||
Count@1002 : Integer;
|
||||
Vendor@1001 : Record 23;
|
||||
Count@1002 : Integer;
|
||||
BEGIN
|
||||
Count := 0;
|
||||
Vendor.SETFILTER("No.",FilterStr);
|
||||
IF Vendor.FINDSET THEN
|
||||
REPEAT
|
||||
"User ID" := USERID;
|
||||
"Vendor No." := Vendor."No.";
|
||||
IF INSERT THEN
|
||||
Count += 1;
|
||||
UNTIL Vendor.NEXT = 0;
|
||||
Count := 0;
|
||||
Vendor.SETFILTER("No.",FilterStr);
|
||||
IF Vendor.FINDSET THEN
|
||||
REPEAT
|
||||
"User ID" := USERID;
|
||||
"Vendor No." := Vendor."No.";
|
||||
IF INSERT THEN
|
||||
Count += 1;
|
||||
UNTIL Vendor.NEXT = 0;
|
||||
END;
|
||||
|
||||
Good code
|
||||
|
||||
PROCEDURE AddEntities@1(FilterStr@1000 : Text\[250\]);
|
||||
PROCEDURE AddEntities@1(FilterStr@1000 : Text[250]);
|
||||
VAR
|
||||
Vendor@1001 : Record 23;
|
||||
Vendor@1001 : Record 23;
|
||||
BEGIN
|
||||
Vendor.SETFILTER("No.",FilterStr);
|
||||
IF Vendor.FINDSET THEN
|
||||
REPEAT
|
||||
"User ID" := USERID;
|
||||
"Vendor No." := Vendor."No.";
|
||||
IF INSERT THEN;
|
||||
UNTIL Vendor.NEXT = 0;
|
||||
Vendor.SETFILTER("No.",FilterStr);
|
||||
IF Vendor.FINDSET THEN
|
||||
REPEAT
|
||||
"User ID" := USERID;
|
||||
"Vendor No." := Vendor."No.";
|
||||
IF INSERT THEN;
|
||||
UNTIL Vendor.NEXT = 0;
|
||||
END;
|
||||
|
|
|
|||
|
|
@ -8,37 +8,37 @@ Unused variables affect readability.
|
|||
|
||||
Bad code
|
||||
|
||||
PROCEDURE CheckPostingDate@23(CaptionEntryNo@1005 : Text\[50\]);
|
||||
PROCEDURE CheckPostingDate@23(CaptionEntryNo@1005 : Text[50]);
|
||||
BEGIN
|
||||
IF GenJnlCheckLine.DateNotAllowed(PostingDate) THEN
|
||||
ERROR(DateNotAllowedErr,Caption,EntryNo)
|
||||
IF PostingDate \> MaxPostingDate THEN
|
||||
MaxPostingDate := PostingDate;
|
||||
IF GenJnlCheckLine.DateNotAllowed(PostingDate) THEN
|
||||
ERROR(DateNotAllowedErr,Caption,EntryNo)
|
||||
IF PostingDate > MaxPostingDate THEN
|
||||
MaxPostingDate := PostingDate;
|
||||
END
|
||||
|
||||
Good code
|
||||
|
||||
PROCEDURE CheckPostingDate@23();
|
||||
BEGIN
|
||||
IF GenJnlCheckLine.DateNotAllowed(PostingDate) THEN
|
||||
ERROR(DateNotAllowedErr,Caption,EntryNo);
|
||||
IF PostingDate \> MaxPostingDate THEN
|
||||
MaxPostingDate := PostingDate;
|
||||
IF GenJnlCheckLine.DateNotAllowed(PostingDate) THEN
|
||||
ERROR(DateNotAllowedErr,Caption,EntryNo);
|
||||
IF PostingDate > MaxPostingDate THEN
|
||||
MaxPostingDate := PostingDate;
|
||||
END;
|
||||
|
||||
Bad code
|
||||
|
||||
PROCEDURE IsReturned@14(EntryNo@1002 : Integer) : Decimal;
|
||||
VAR
|
||||
ItemEntry@1000 : Record 32;
|
||||
Quantity@1003 : Integer;
|
||||
ItemEntry@1000 : Record 32;
|
||||
Quantity@1003 : Integer;
|
||||
BEGIN
|
||||
EXIT(-OutboundApplied(EntryNo,TRUE) - InboundApplied(EntryNo,TRUE));
|
||||
EXIT(-OutboundApplied(EntryNo,TRUE) - InboundApplied(EntryNo,TRUE));
|
||||
END;
|
||||
|
||||
Good code
|
||||
|
||||
PROCEDURE IsReturned@14(EntryNo@1002 : Integer) : Decimal;
|
||||
BEGIN
|
||||
EXIT(-OutboundApplied(EntryNo,TRUE) - InboundApplied(EntryNo,TRUE));
|
||||
EXIT(-OutboundApplied(EntryNo,TRUE) - InboundApplied(EntryNo,TRUE));
|
||||
END;
|
||||
|
|
|
|||
|
|
@ -8,52 +8,52 @@ It will throw an error at runtime.
|
|||
|
||||
Bad code
|
||||
|
||||
FileName@1010 : Text\[250\];
|
||||
FileName@1010 : Text[250];
|
||||
...
|
||||
UploadedFileName@1016 : Text\[1024\];
|
||||
UploadedFileName@1016 : Text[1024];
|
||||
...
|
||||
FileName := UploadedFileName;
|
||||
|
||||
Good code
|
||||
|
||||
FileName@1010 : Text\[1024\];
|
||||
FileName@1010 : Text[1024];
|
||||
...
|
||||
UploadedFileName@1016 : Text\[1024\];
|
||||
UploadedFileName@1016 : Text[1024];
|
||||
...
|
||||
FileName := UploadedFileName;
|
||||
|
||||
Bad code
|
||||
|
||||
FileName@1010 : Text\[250\];
|
||||
FileName@1010 : Text[250];
|
||||
...
|
||||
UploadedFileName@1016 : Text\[1024\];
|
||||
UploadedFileName@1016 : Text[1024];
|
||||
...
|
||||
FileName := UploadedFileName;
|
||||
|
||||
Good code
|
||||
|
||||
FileName@1010 : Text\[250\];
|
||||
FileName@1010 : Text[250];
|
||||
...
|
||||
UploadedFileName@1016 : Text\[1024\];
|
||||
UploadedFileName@1016 : Text[1024];
|
||||
...
|
||||
FileName := COPYSTR(UploadedFileName,1,250); // In case only the first 250 chars are needed. Similar for fields
|
||||
|
||||
Bad code
|
||||
|
||||
VAR
|
||||
ExceededNumberTxt@001 : 'ENU=Warning: Exceeded number of unsent documents/requests'
|
||||
Subject@1002 : Text\[50\];
|
||||
...
|
||||
ExceededNumberTxt@001 : 'ENU=Warning: Exceeded number of unsent documents/requests'
|
||||
Subject@1002 : Text[50];
|
||||
...
|
||||
BEGIN
|
||||
...
|
||||
Subject := ExceededNumberTxt;
|
||||
...
|
||||
Subject := ExceededNumberTxt;
|
||||
|
||||
Good code
|
||||
|
||||
VAR
|
||||
ExceededNumberTxt@001 : 'ENU=Warning: Exceeded number of unsent documents/requests'
|
||||
Subject@1002 : Text\[100\];
|
||||
...
|
||||
ExceededNumberTxt@001 : 'ENU=Warning: Exceeded number of unsent documents/requests'
|
||||
Subject@1002 : Text[100];
|
||||
...
|
||||
BEGIN
|
||||
...
|
||||
Subject := ExceededNumberTxt';
|
||||
...
|
||||
Subject := ExceededNumberTxt';
|
||||
|
|
|
|||
|
|
@ -4,17 +4,18 @@ weight = 1450
|
|||
+++
|
||||
Do not use the WITH scope when it has a variable whose name is the same as a local variable. This can lead to wrong code assumptions.
|
||||
|
||||
**Given that** "Contract Type" is a field on table ServiceContractHeader, then in the following example there is a parameter name clash with the field name. Which one will be used?
|
||||
**Given that**
|
||||
"Contract Type" is a field on table ServiceContractHeader, then in the following example there is a parameter name clash with the field name. Which one will be used?
|
||||
|
||||
Bad code
|
||||
|
||||
PROCEDURE InsertData@1("Contract Type"@1000 : Option...);
|
||||
...
|
||||
BEGIN
|
||||
...
|
||||
WITH ServiceContractHeader DO BEGIN
|
||||
...
|
||||
DimMgt.InsertServContractDim(...,"Contract Type","Contract No.",0,...);
|
||||
...
|
||||
WITH ServiceContractHeader DO BEGIN
|
||||
...
|
||||
DimMgt.InsertServContractDim(...,"Contract Type","Contract No.",0,...);
|
||||
END;
|
||||
|
||||
Good code
|
||||
|
|
@ -22,8 +23,8 @@ Good code
|
|||
PROCEDURE InsertData@1(ContractType@1000 : Option...);
|
||||
...
|
||||
BEGIN
|
||||
...
|
||||
WITH ServiceContractHeader DO BEGIN
|
||||
...
|
||||
DimMgt.InsertServContractDim(...,ContractType,"Contract No.",0,...);
|
||||
...
|
||||
WITH ServiceContractHeader DO BEGIN
|
||||
...
|
||||
DimMgt.InsertServContractDim(...,ContractType,"Contract No.",0,...);
|
||||
END;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue