Dynamics Community Forum Thread Details

Coding Guidelines: Single lines, BEGIN/END and Readability

Hello,

as already commented on the blog entry regarding community guidelines, I don't agree with everything (although I really appreciate the effort! It's great, thank you! =)).

One particular thing I don't like is the use (or better lack of use) of BEGIN and END to enclose statements. MS recommends, that we only use it, when we need more than one statement. While I do not like this rule at all, I can understand its charme. But only if it is used with single line or easy to understand statements. Today I found this code in Table 39 Purchase Line and it drove me mad:

IF Quantity = 0 THEN BEGIN
  ...
END ELSE BEGIN
  GetPurchHeader;
  IF PurchHeader.Status = PurchHeader.Status::Released THEN
    AmountInclVAT := "Amount Including VAT"
  ELSE
    IF PurchHeader."Prices Including VAT" THEN
      AmountInclVAT := "Line Amount" - "Inv. Discount Amount"
    ELSE
      IF "VAT Calculation Type" = "VAT Calculation Type"::"Sales Tax" THEN BEGIN
        IF "Use Tax" THEN
          AmountInclVAT := "Line Amount" - "Inv. Discount Amount"
        ELSE
          AmountInclVAT :=
            "Line Amount" - "Inv. Discount Amount" +
            ROUND(
              SalesTaxCalculate.CalculateTax(
                "Tax Area Code","Tax Group Code","Tax Liable",PurchHeader."Posting Date",
                "Line Amount" - "Inv. Discount Amount","Quantity (Base)",PurchHeader."Currency Factor"),
              Currency."Amount Rounding Precision")
      END ELSE
        AmountInclVAT :=
          ROUND(
            ("Line Amount" - "Inv. Discount Amount") *
            (1 + "VAT %" / 100 * (1 - PurchHeader."VAT Base Discount %" / 100)),
            Currency."Amount Rounding Precision");
 VALIDATE(
   "Outstanding Amount",
   ROUND(
     AmountInclVAT * "Outstanding Quantity" / Quantity,
     Currency."Amount Rounding Precision"));
...

There are open ended IFs and ELSEs, which again call IFs and ELSEs and so on. It is really hard to read this code and to top it all off, there are statement calls broken into multiple lines to optimise the readablity of these particular lines while making it worse to identify where the IF-statements began and end. Especially the connection of the last VALIDATE statement is not that easy to identify. It would be much clearer if it would be like this:

IF Quantity = 0 THEN BEGIN
  ...
END ELSE BEGIN
  GetPurchHeader;
  IF PurchHeader.Status = PurchHeader.Status::Released THEN
    AmountInclVAT := "Amount Including VAT"
  ELSE BEGIN
    IF PurchHeader."Prices Including VAT" THEN
      AmountInclVAT := "Line Amount" - "Inv. Discount Amount"
    ELSE BEGIN
      IF "VAT Calculation Type" = "VAT Calculation Type"::"Sales Tax" THEN BEGIN
        IF "Use Tax" THEN
          AmountInclVAT := "Line Amount" - "Inv. Discount Amount"
        ELSE BEGIN
          AmountInclVAT :=
            "Line Amount" - "Inv. Discount Amount" +
            ROUND(
              SalesTaxCalculate.CalculateTax(
                "Tax Area Code","Tax Group Code","Tax Liable",PurchHeader."Posting Date",
                "Line Amount" - "Inv. Discount Amount","Quantity (Base)",PurchHeader."Currency Factor"),
              Currency."Amount Rounding Precision")
        END;
      END
      ELSE BEGIN
        AmountInclVAT :=
          ROUND(
            ("Line Amount" - "Inv. Discount Amount") *
            (1 + "VAT %" / 100 * (1 - PurchHeader."VAT Base Discount %" / 100)),
            Currency."Amount Rounding Precision");
      END;
  END;

  VALIDATE(
    "Outstanding Amount",
    ROUND(
      AmountInclVAT * "Outstanding Quantity" / Quantity,
      Currency."Amount Rounding Precision"));
  ...
END;

What do you think?

Cheers,
Patrik

Quick Links

Leaderboard