Restructure for Docsy theme
This commit is contained in:
Jeremy Vyska
parent
f4a1ebc077
commit
cceb0c945e
376 changed files with 963 additions and 186 deletions
|
|
@ -0,0 +1,9 @@
|
|||
+++
|
||||
title = "Design"
|
||||
weight = 490
|
||||
+++
|
||||
## C/AL Coding Guidelines
|
||||
|
||||
## **Design**
|
||||
|
||||
Find the C/AL guidelines by expanding the menu in the left.
|
||||
|
|
@ -0,0 +1,25 @@
|
|||
+++
|
||||
title = "By Reference Parameters"
|
||||
weight = 280
|
||||
+++
|
||||
Do not declare parameters by reference if their values are not intended to be changed.
|
||||
|
||||
Unintentional value changes might propagate. Also, it might lead people to believe that value changes are intended.
|
||||
|
||||
Bad code
|
||||
|
||||
LOCAL PROCEDURE ShowMessage@15(VAR Text@1000 : Text[250]);
|
||||
BEGIN
|
||||
Text := GetMessageText;
|
||||
IF (Text <> '') AND GenJnlLineInserted THEN
|
||||
MESSAGE(Text);
|
||||
END;
|
||||
|
||||
Good code
|
||||
|
||||
LOCAL PROCEDURE ShowMessage@15(Text@1000 : Text[250]);
|
||||
BEGIN
|
||||
Text := GetMessageText;
|
||||
IF (Text <> '') AND GenJnlLineInserted THEN
|
||||
MESSAGE(Text);
|
||||
END;
|
||||
|
|
@ -0,0 +1,18 @@
|
|||
+++
|
||||
title = "Class Coupling"
|
||||
weight = 320
|
||||
+++
|
||||
Do not write functions that have high class coupling. This makes the code hard to maintain.
|
||||
|
||||
Bad code
|
||||
|
||||
Any procedure / trigger that has class coupling of > 30
|
||||
|
||||
|
||||
|
||||
Good code
|
||||
|
||||
Any procedure / trigger that has class coupling of <= 30\.
|
||||
Class coupling is computed by summing the unique instances of the following in a code block:
|
||||
- every unique usage of a complex C/AL data type (table, codeunit, etc) as 1\.
|
||||
- every unique usage of a DotNet type as 1\.
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
+++
|
||||
title = "Cyclomatic Complexity"
|
||||
weight = 460
|
||||
+++
|
||||
Do not write functions that have high cyclomatic complexity. This makes the code hard to maintain.
|
||||
|
||||
Bad code
|
||||
|
||||
Any procedure / trigger that has a cyclomatic complexity > 25, using the CC3 version mentioned in [this article][anchor0].
|
||||
|
||||
|
||||
|
||||
Good code
|
||||
|
||||
Any procedure / trigger that has a cyclomatic complexity <= 25, using the CC3 version.
|
||||
The CC3 version is computed by summing the following in a code block:
|
||||
- each IF statement as 1\.
|
||||
- each entire CASE as 1\.
|
||||
|
||||
|
||||
|
||||
[anchor0]: http://www.aivosto.com/project/help/pm-complexity.html
|
||||
|
|
@ -0,0 +1,5 @@
|
|||
+++
|
||||
title = "Encapsulate Local Functionality"
|
||||
weight = 530
|
||||
+++
|
||||
Any function used local must be defined as local.
|
||||
|
|
@ -0,0 +1,29 @@
|
|||
+++
|
||||
title = "FINDSET FINDFIRST FINDLAST"
|
||||
weight = 600
|
||||
+++
|
||||
FINDSET, FIND('+') or FIND('-') should only be used when NEXT is used and vice versa.
|
||||
|
||||
Bad code
|
||||
|
||||
IF Cust.FIND('-') THEN
|
||||
ERROR(CustIsBlockErr)
|
||||
|
||||
Good code
|
||||
|
||||
IF Cust.FINDFIRST THEN
|
||||
ERROR(CustIsBlockErr)
|
||||
|
||||
Bad code
|
||||
|
||||
IF Cust.FINDFIRST THEN
|
||||
REPEAT
|
||||
...
|
||||
UNTIL Cust.NEXT = 0;
|
||||
|
||||
Good code
|
||||
|
||||
IF Cust.FINDSET THEN
|
||||
REPEAT
|
||||
...
|
||||
UNTIL Cust.NEXT = 0;
|
||||
|
|
@ -0,0 +1,49 @@
|
|||
+++
|
||||
title = "Initialized Variables"
|
||||
weight = 660
|
||||
+++
|
||||
Variables should always be set to a specific value, before they are used.
|
||||
|
||||
Bad code
|
||||
|
||||
PROCEDURE SetPurchLine@22(VAR CurrentPurchLine@1000 : Record 39);
|
||||
VAR
|
||||
Pegging@1001 : Boolean;
|
||||
BEGIN
|
||||
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;
|
||||
BEGIN
|
||||
Pegging := IsPegging(CurrentPurchLine);
|
||||
IF Pegging THEN
|
||||
CurrQuantity := CurrentPurchLine."Quantity (Base)"
|
||||
ELSE
|
||||
CurrQuantity := CurrentPurchLine."Outstanding Qty. (Base)";
|
||||
END;
|
||||
|
||||
Bad code
|
||||
|
||||
// In the example below, the function will always return FALSE.
|
||||
PROCEDURE GetItemsToPlan@22() : Boolean;
|
||||
BEGIN
|
||||
SETRANGE("Document Type","Document Type"::Order);
|
||||
...
|
||||
FINDSET
|
||||
END;
|
||||
|
||||
Good code
|
||||
|
||||
PROCEDURE GetItemsToPlan@22() : Boolean;
|
||||
BEGIN
|
||||
SETRANGE("Document Type","Document Type"::Order);
|
||||
...
|
||||
EXIT(FINDSET)
|
||||
END;
|
||||
|
|
@ -0,0 +1,41 @@
|
|||
+++
|
||||
title = "Maintainability Index"
|
||||
weight = 770
|
||||
+++
|
||||
[Maintainability Index][anchor0]: Do not write functions that have a very low maintainability index. This makes the code hard to maintain.
|
||||
|
||||
Bad code
|
||||
|
||||
Any procedure / trigger that has a maintainability index < 20
|
||||
|
||||
|
||||
|
||||
Good code
|
||||
|
||||
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).
|
||||
|
||||
More info
|
||||
|
||||
* [Halstead Volume][anchor1]
|
||||
* [Cyclomatic Complexity][anchor2]
|
||||
|
||||
Bad code
|
||||
|
||||
Any procedure / trigger that is > 100 lines of code
|
||||
|
||||
|
||||
|
||||
Good code
|
||||
|
||||
Any procedure / trigger that is <= 100 lines of code.
|
||||
A full C/AL Statement counts as 1 line of code
|
||||
|
||||
|
||||
|
||||
[anchor0]: http://blogs.msdn.com/b/codeanalysis/archive/2007/11/20/maintainability-index-range-and-meaning.aspx
|
||||
[anchor1]: http://en.wikipedia.org/wiki/Halstead_complexity_measures
|
||||
[anchor2]: http://www.aivosto.com/project/help/pm-complexity.html
|
||||
|
|
@ -0,0 +1,33 @@
|
|||
+++
|
||||
title = "Parameter Placeholders"
|
||||
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.';
|
||||
...
|
||||
ERROR(CannotDeleteLineErr,TABLECAPTION);
|
||||
|
||||
Good code
|
||||
|
||||
CannotDeleteLineErr@1005 : TextConst 'ENU=You cannot delete this line because one or more rating values exists.';
|
||||
...
|
||||
ERROR(CannotDeleteLineErr);
|
||||
|
||||
|
||||
|
||||
Bad code
|
||||
|
||||
CannotUseThisFieldErr@1020 : TextConst 'ENU=You cannot use this field for %2 fields.';
|
||||
...
|
||||
ERROR(CannotUseThisFieldErr,0,Field.Class);
|
||||
|
||||
Good code
|
||||
|
||||
CannotUseThisFieldErr@1020 : TextConst 'ENU=You cannot use this field for %1 fields.';
|
||||
...
|
||||
ERROR(CannotUseThisFieldErr,Field.Class);
|
||||
|
||||
###
|
||||
|
|
@ -0,0 +1,21 @@
|
|||
+++
|
||||
title = "Static Object Invocation"
|
||||
weight = 1160
|
||||
+++
|
||||
Call objects statically whenever possible. It reduces extra noise and removes extra variables. Downside: changing the name of the object which is called statically will need a code update.
|
||||
|
||||
Bad code
|
||||
|
||||
LOCAL PROCEDURE Code@1();
|
||||
VAR
|
||||
CAJnlPostBatch@1001 : Codeunit 1103;
|
||||
BEGIN
|
||||
CAJnlPostBatch.Run(CostJnlLine);
|
||||
END;
|
||||
|
||||
Good code
|
||||
|
||||
LOCAL PROCEDURE Code@1();
|
||||
BEGIN
|
||||
CODEUNIT.RUN(CODEUNIT::"CA Jnl.-Post Batch",CostJnlLine);
|
||||
END;
|
||||
|
|
@ -0,0 +1,25 @@
|
|||
+++
|
||||
title = "Unreachable Code"
|
||||
weight = 1310
|
||||
+++
|
||||
Do not write code that will never be hit.
|
||||
|
||||
It affects code readability and can lead to wrong assumptions.
|
||||
|
||||
Bad code
|
||||
|
||||
IF Type <> Type::FIELD THEN BEGIN
|
||||
...
|
||||
ERROR(...);
|
||||
RecRef.CLOSE;
|
||||
END;
|
||||
|
||||
|
||||
|
||||
Good code
|
||||
|
||||
IF Type <> Type::FIELD THEN BEGIN
|
||||
...
|
||||
RecRef.CLOSE;
|
||||
ERROR(...);
|
||||
END;
|
||||
|
|
@ -0,0 +1,38 @@
|
|||
+++
|
||||
title = "Unused Initialized Variables"
|
||||
weight = 1320
|
||||
+++
|
||||
The value assigned to a variable must be used. Else the variable is not necessary.
|
||||
|
||||
Bad code
|
||||
|
||||
PROCEDURE AddEntities@1(FilterStr@1000 : Text[250]);
|
||||
VAR
|
||||
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;
|
||||
END;
|
||||
|
||||
Good code
|
||||
|
||||
PROCEDURE AddEntities@1(FilterStr@1000 : Text[250]);
|
||||
VAR
|
||||
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;
|
||||
END;
|
||||
|
|
@ -0,0 +1,44 @@
|
|||
+++
|
||||
title = "Unused Variables"
|
||||
weight = 1330
|
||||
+++
|
||||
Do not declare variables that are unused.
|
||||
|
||||
Unused variables affect readability.
|
||||
|
||||
Bad code
|
||||
|
||||
PROCEDURE CheckPostingDate@23(CaptionEntryNo@1005 : Text[50]);
|
||||
BEGIN
|
||||
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;
|
||||
END;
|
||||
|
||||
Bad code
|
||||
|
||||
PROCEDURE IsReturned@14(EntryNo@1002 : Integer) : Decimal;
|
||||
VAR
|
||||
ItemEntry@1000 : Record 32;
|
||||
Quantity@1003 : Integer;
|
||||
BEGIN
|
||||
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));
|
||||
END;
|
||||
|
|
@ -0,0 +1,59 @@
|
|||
+++
|
||||
title = "Variable Capacity Mismatch"
|
||||
weight = 1410
|
||||
+++
|
||||
Do not assign a value to a variable whose capacity is smaller.
|
||||
|
||||
It will throw an error at runtime.
|
||||
|
||||
Bad code
|
||||
|
||||
FileName@1010 : Text[250];
|
||||
...
|
||||
UploadedFileName@1016 : Text[1024];
|
||||
...
|
||||
FileName := UploadedFileName;
|
||||
|
||||
Good code
|
||||
|
||||
FileName@1010 : Text[1024];
|
||||
...
|
||||
UploadedFileName@1016 : Text[1024];
|
||||
...
|
||||
FileName := UploadedFileName;
|
||||
|
||||
Bad code
|
||||
|
||||
FileName@1010 : Text[250];
|
||||
...
|
||||
UploadedFileName@1016 : Text[1024];
|
||||
...
|
||||
FileName := UploadedFileName;
|
||||
|
||||
Good code
|
||||
|
||||
FileName@1010 : Text[250];
|
||||
...
|
||||
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];
|
||||
...
|
||||
BEGIN
|
||||
...
|
||||
Subject := ExceededNumberTxt;
|
||||
|
||||
Good code
|
||||
|
||||
VAR
|
||||
ExceededNumberTxt@001 : 'ENU=Warning: Exceeded number of unsent documents/requests'
|
||||
Subject@1002 : Text[100];
|
||||
...
|
||||
BEGIN
|
||||
...
|
||||
Subject := ExceededNumberTxt';
|
||||
|
|
@ -0,0 +1,30 @@
|
|||
+++
|
||||
title = "WITH Scope Name Collision"
|
||||
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?
|
||||
|
||||
Bad code
|
||||
|
||||
PROCEDURE InsertData@1("Contract Type"@1000 : Option...);
|
||||
...
|
||||
BEGIN
|
||||
...
|
||||
WITH ServiceContractHeader DO BEGIN
|
||||
...
|
||||
DimMgt.InsertServContractDim(...,"Contract Type","Contract No.",0,...);
|
||||
END;
|
||||
|
||||
Good code
|
||||
|
||||
PROCEDURE InsertData@1(ContractType@1000 : Option...);
|
||||
...
|
||||
BEGIN
|
||||
...
|
||||
WITH ServiceContractHeader DO BEGIN
|
||||
...
|
||||
DimMgt.InsertServContractDim(...,ContractType,"Contract No.",0,...);
|
||||
END;
|
||||
Loading…
Add table
Add a link
Reference in a new issue