Correction guide - Compiler warnings
The following is based on material kindly provided by Michael Fruergaard Pontoppidan. I have edited it substantially and added some new material to make it fit within the context of this blog.
Introduction
This guide will show you how to resolve compiler warnings in Axapta.
Compiler warnings are a strong indication that the code contains flaws or questionable constructs. The compiler groups the warnings in several levels according to the warning's severity: Level 1 is for very questionable issues, downto level 4 which are smaller issues. All compiler warnings must all be eliminated from production code to improve the overall code quality.
Note that the Dynamics Ax development environment also features a best practise framework that diagnoses the code's adherence to the established best practises. The diagnostics issued by this tool are not covered here.
Contents
Break statement found outside legal context.
The new method of a derived class does not call super().
The new method of a derived class may not call super().
Function never returns a value.
Assignment/Comparison loses precision.
Updating Views is not supported.
Class names should start with an uppercase letter.
Member names should start with a lowercase letter.
Compiler Warnings Level 1
Break statement found outside legal context
Break statements are used to skip to the end of for, while, do-while and switch statements. If a break statement is encountered outside the body of one of these statements it is ignored at runtime and should be removed from the code to avoid confusion and improve legibility.
Example
1: {
2: int i;
3:
4: for (i = 1; i <= 100; i++)
5: {
6: foo(i);
7: if (i==8)
8: break;
9: }
10: if (i==8)<br>11: break;
12: }
Fix:
Move or remove the break statement to properly reflect the intention.
1: {
2: int i;
3:
4: for (i = 1; i <= 100; i++)
5: {
6: foo(i);
7: if (i==8)
8: break;
9: }
10: }
The new method of a derived class does not call super()
Always call super() in the constructor (i.e. the new method) of a class: The parent class could have some initialization of its own to do. You should do this even if the parent class implementation does not currently set up any state, because the implementation may subsequently be changed. Failure to call the parent's initialization code will typically break the base class.
Example
This class is derived from the Runbase class. If you don't call super() in this new method the code in the inherited new method will never be executed, depriving the Runbase framework of being properly initialized.
1: class MyClass extends Runbase
2: {
3: void new()
4: {
5: ;
6: emplId = EmplTable::userId2EmplId(curUserId());
7: }
8: }
Fix:
1: class MyClass extends Runbase
2: {
3: void new()
4: {
5: ;
6: super(); // Initialize base class.
7: emplId = EmplTable::userId2EmplId(curUserId());
8: }
9: }
The new method of a derived class may not call super()
Always call super() in the constructor (i.e. the new method) of a class. The parent class could have some initialisation of its own to do. You should do this even if the parent class implementation does not currently set up any state, because the implementation may subsequently be changed.
Note that this message is different from the previous one: In this case the compiler can prove that super will not always be called (i.e. there are paths in the code that cause the method to end without calling super). In the previous case the compiler can prove that no paths cause super() to be executed.
Example
This class is inherited from the Runbase class. If you don't always call super() in this new method the code in the inherited new method will never be executed.
1: class MyClass extends Runbase
2: {
3: void new()
4: {
5: ;
6: if (this.init())
7: super(); // Not called if init() fails.
8: emplId = EmplTable::userId2EmplId(curUserId());
9: }
10: }
Fix:
Always call super as shown below:
1: class MyClass extends Runbase
2: {
3: void new()
4: {
5: ;
6: this.init();
7: super(); // Always called.
8: emplId = EmplTable::userId2EmplId(curUserId());
9: }
10: }
Function never returns a value
Any non abstract method with a return type (i.e. a method that does not return void) must explicitly return a value, irrespective of which path the execution takes through the code. If this is not the case, the warning above is issued.
Sometimes programmers write methods that have no implementation (and therefore no return statements) because they expect that derived classes should provide an implementation. In this case, an abstract class or an interface should be used, because this causes the compiler to check that all abstract methods (or interface methods) are actually overridden by a derived class.
Example
The user has written the following code because he expects the programmer to define a more meaningful implementation in derived classes. If applicable, the method should be made abstract forcing derived classes to implement this method.
1: abstract class MyClass
2: {
3: InventQty onHandQty(InventOnHand inventOnHand)
4: {
5: }
6: }
Fix:
1: abstract class MyClass
2: {
3: abstract InventQty onHandQty(InventOnHand inventOnHand)
4: {
5: // No implementation. Implementations must be provided
6: // by non abstract derived classes.
7: }
8: }
Not all paths return a value
The method has a return value (i.e. it is not declared as void) and it is possible for the method to end without executing a return statement. This is never a problem for methods that do not return values (i.e. are declared as void) because they always return when the last statement is executed. This is a warning that invariably indicates a flaw in the code.
This warning is different from the one described above: In this case the compiler can prove that there are paths that do not return a value. The warning message above is issued if the compiler can prove that there are no paths that return a value.
Example
The switch statement below contains case entries for some of the enumeration literals in the PurchaseType enumeration type. However, if the purchaseType parameter does not contain one of the values provided for in the cases, no case entry would be selected and the method would end without explicitly returning a value. Even if all the values provided in the enumeration were dealt with (by providing a case entry for each literal) the compiler would still issue the warning message.
1: static PurchCreateOrderForm construct(PurchaseType purchaseType, NoYesId project)
2: {
3: switch (purchaseType)
4: {
5: case PurchaseType::Journal:
6: return PurchCreateOrderForm_Journal::construct(purchaseType, project);
7: case PurchaseType::Quotation:
8: return PurchCreateOrderForm_Quotation::construct(purchaseType, project);
9: case PurchaseType::Purch:
10: return PurchCreateOrderForm_Purch::construct(purchaseType, project);
11: case PurchaseType::Subscription:
12: return PurchCreateOrderForm_Subscription::construct(purchaseType, project);
13: }
14: }
Fix:
In the case below, the problem could be resolved by always returning after the switch statement.
1: static PurchCreateOrderForm construct(PurchaseType purchaseType, NoYesId project)
2: {
3: switch (purchaseType)
4: {
5: case PurchaseType::Journal:
6: return PurchCreateOrderForm_Journal::construct(purchaseType, project);
7: case PurchaseType::Quotation:
8: return PurchCreateOrderForm_Quotation::construct(purchaseType, project);
9: case PurchaseType::Purch:
10: return PurchCreateOrderForm_Purch::construct(purchaseType, project);
11: case PurchaseType::Subscription:
12: return PurchCreateOrderForm_Subscription::construct(purchaseType, project);
13: }
14: return new PurchCreateOrderForm();
15: }
Another approach is to provide a default handler to catch the items not explicitly dealt with.
1: static PurchCreateOrderForm construct(PurchaseType purchaseType, NoYesId project)
2: {
3: switch (purchaseType)
4: {
5: case PurchaseType::Journal:
6: return PurchCreateOrderForm_Journal::construct(purchaseType, project);
7: case PurchaseType::Quotation:
8: return PurchCreateOrderForm_Quotation::construct(purchaseType, project);
9: case PurchaseType::Purch:
10: return PurchCreateOrderForm_Purch::construct(purchaseType, project);
11: case PurchaseType::Subscription:
12: return PurchCreateOrderForm_Subscription::construct(purchaseType, project);
13: default:<br>14: return new PurchCreateOrderForm();
15: }
16: }
Assignment/Comparison loses precision
This warning is issued when an implicit conversion is performed whereby the value is modified. This happens in these scenarios:
- Real to int
- Int64 to int
Real to int
Thye problem occurs when a real is being assigned to or compared to a value fo an integer type. This may be correct, but it should be made explicit that the value is truncated, not rounded. Also, unexpected results may happen when the real (floating point) value is greater than what is representable in a value of the given integer type (i.e. outside the range of (-2.147.483.678; 2.147.483.677) for 32 bit integers and (-9223372036854775808; 9223372036854775807) for 64 bit integers).
The global method real2int(real) can be used in most cases, providing truncation.
Example
The variable scrapConst is declared as int in the example below. When executing the assignment statement scrapConst = BOMMap.scrapConst() a type conversion from real to int is performed, because the resulting type from the method call is real.
1: container consumptionTypeItem (BOMMap BOM)
2: {
3: BOMMap BOMMap;
4: int scrapConst;
5: ;
6: BOMMap = BOM.data();
7: scrapConst = BOMMap.scrapConst();
8:
9: // ...
10: }
Fix:
The fix depends on what you want to achieve; there are more solutions to this case. If you want to rounding to take place (instead of truncation) you should use the round function before assigning the value to the integer. Typically the proper fix is to declare scrapConst as a real.
Int64 to int
Axapta 4.0 natively supports 64 bit integers. The problem occurs when trying to assign a value of type int64 to a variable of type int, or comparing int64 values with int values.
Example
In the example below, the value assigned to j is maxInt() and can just fit into a signed 32-bit integer.
The value assigned to i equals maxInt()+1 and can not fit into a signed 32-bit integer, so Axapta will choose a 64 bit integer. Converting an int64 to an int gives the warning because a 64 bit value is not generally representable in 32 bits.
1: {
2: int i = 0x7fffffff;
3: int j = 0x80000000;
4: int k = 0x80000000u;
5: }
Fix:
Depending on what you want you can either choose to use a int64, which can safely hold the value, or place the value into an int, losing the most significant 32 bits. Note that X++ features syntax where a U can be appended to the integer literal to indicate that the value should be interpreted as an unsigned value. The third assignment in the example above will not generate a warning message because 0x80000000u is interpreted as an unsigned int. However, there are no unsigned types in X++; all arithmetic is signed.
You can use the any2int function if you want to go ahead and disregard the most significant bits.
1: {
2: int i = any2int(0x80000000);
3: int64 j = 0x80000000;
4: }
Compiler Warnings Level 2
Unreachable code
Unreachable code is code fragments that will never be executed, as the code flow prevents the code from being used. This warning is issued for code that is placed immediately after statements that unconditionally change the flow of control, i.e. the return, break, continue and throw statements.
Example
The break statement will never be executed as the return will exit the method.
1: static BOMRouteApprove construct(Args args)
2: {
3: BOMRouteApprove approve;
4:
5: switch (args.dataset())
6: {
7: case TableNum(routeTable):
8: return RouteApprove::newRouteTable(args.record());
9: break; // Execution never reaches this.
10: default:
11: throw error(strfmt("@SYS29104", classId2Name(classIdGet(approve))));
12: }
13: }
Fix:
Simply remove the statement that is never executed:
1: static BOMRouteApprove construct(Args args)
2: {
3: BOMRouteApprove approve;
4:
5: switch (args.dataset())
6: {
7: case TableNum(routeTable):
8: return RouteApprove::newRouteTable(args.record());
10: default:
11: throw error(strfmt("@SYS29104", classId2Name(classIdGet(approve))));
12: }
13: }
Compiler Warnings Level 3
Empty compound statement
A compound statement is a statement that contains any number of other statements so that they are precieved by the compiler as a being single statement. A compound statement is constructed by prefixing the statement list with a { character and suffixing the statement list with }. An empty compound statement, then, is simply a compound statement containg no statements. This is either a sign of sloppy code and should be removed, or (worse) a sign of missing functionality that the programmer was intending to put in later. In that case, the missing functionality should be implemented or a //ToDo comment should be inserted. The //ToDo will not remove the compiler warning, as it is just a comment, but it will help the programmer remember to fill in the code.
One situation exists, as shown below, where it makes sence to have an empty compond statement. This happens when an exception must be caught without any explict functionality to be performed as a result. The exception handler must be provided to make sure it is not thrown in the calling context.
Example
The empty catch statement doesn't clean up the infolog, so the exception text falls through.
1: void run()
2: {
3: BOMParmReportFinish BOMParmReportFinish;
4: BOMReportFinish BOMReportFinish;
5:
6: if (!this.validate())
7: throw error("@SYS18447");
8:
9: select BOMParmReportFinish
10: index ParmLineNumIdx
11: where BomParmReportFinish.parmId == parmId;
12:
13: while (BOMParmReportFinish)
14: {
15: try
16: {
17: BOMReportFinish = BOMReportFinish::newParmBuffer(BOMparmReportFinish, this.postJournalPerBOMParm());
18: BOMReportFinish.run();
19: if (this.isInBatch())
20: BOMReportFinish.journalTableData().updateBlock(JournalBlockLevel::System, JournalBlockLevel::None);
21:
22: this.findCallerRecord();
23: }
24: catch (Exception::Deadlock)
25: {
26: retry;
27: }
28: catch (Exception::Error)<br>29: {<br>30: // Empty catch statement <br>31: }
32: }
33: }
Fix:
Make it explicit that you want this behavoiur by adding a call to exceptionTextFallThorugh() in the catch statement. This method (in the Globals class, so it is available in the global scope) doesn't do anything.
1: void run()
2: {
3: BOMParmReportFinish BOMParmReportFinish;
4: BOMReportFinish BOMReportFinish;
5:
6: if (!this.validate())
7: throw error("@SYS18447");
8:
9: select BOMParmReportFinish
10: index ParmLineNumIdx
11: where BomParmReportFinish.parmId == parmId;
12:
13: while (BOMParmReportFinish)
14: {
15: try
16: {
17: BOMReportFinish = BOMReportFinish::newParmBuffer(BOMparmReportFinish, this.postJournalPerBOMParm());
18: BOMReportFinish.run();
19: if (this.isInBatch())
20: BOMReportFinish.journalTableData().updateBlock(JournalBlockLevel::System, JournalBlockLevel::None);
21:
22: this.findCallerRecord();
23: }
24: catch (Exception::Deadlock)
25: {
26: retry;
27: }
28: catch (Exception::Error)
29: {
30: exceptionTextFallThrough() ;
31: }
32: }
33: }
Updating Views is not supported.
Views provide a way of seeing several tables as set up by a query as one table. These pseudotables are readonly. As such it does not make sense to try to modify the data. If an attempt is made to update the data, by adding the forupdate keyword to the select statement using a view in the from clause as illustrated below, a warning occurs.
1: static void MyJob(Args a)
2: {
3: CustOpenInvoices openInvoices;
4: while select forupdate * from openInvoices
5: {
6: print openInvoices.AmountCur;
7: }
8: pause;
9: }
Fix:
Use the tables that the views "look into" to actually access the data. You may do this by using a query object to access the data, or provide a select statement over the tables sampled by the view query.
Compiler Warnings Level 4
Class names should start with an uppercase letter.
According to best practice conventions all class names must start with a capital letter.
Example
1: public class smmRepairDocuRef extends RunBase
2: {
3: container tableCollectionsInVirtualCompany;
4: }
Fix:
1: public class SmmRepairDocuRef extends RunBase
2: {
3: container tableCollectionsInVirtualCompany;
4: }
Note:
You can run the add-ins tool title case update to solve these issues.
Member names should start with a lowercase letter.
According to best practice conventions member names must start with a lowercase letter.
Example
1: public class MyClass
2: {
3: Runbase Runbase;
4: }
Fix:
1: public class MyClass
2: {
3: Runbase runbase;
4: }
Note:
You can run the add-ins tool title case update to solve these issues.
Comments
Anonymous
September 09, 2007
Regarding compiler warning "Not all paths return a value" and the 2nd fix to this warning: using a default handler. It is my experience that using a default handler in the switch case contruct does not remove the warning: int invoicePeriodDays() { switch (this.InvoicePeriod) { case (ACMIntervalType::None) : return 0; case (ACMIntervalType::Day) : return 1; case (ACMIntervalType::Week) : return 7; case (ACMIntervalType::Month) : return 30; case (ACMIntervalType::Quarter) : return 90; case (ACMIntervalType::Year) : return 365; default : return 0; } } I get the warning on the above shown code, even though I am using a default handler. And I've seen this warning on similar code snippets many times. It is as if the BP tool evaluates that there is one path left where the code can return something - being just before the closing curly bracket of the method.Anonymous
September 09, 2007
The warning is also issued on the following construct: Which you could argue is correct (?) as the default handler does not return anything static APMStageHandler construct(common _common) { switch(_common.tableId) { case (tableNum(APMJobTable)): return new APMStageHandler_JobTable(_common); case (tableNum(LedgerJournalTrans)): return new APMStageHandler_LedgerJournalTrans(_common); case (tableNum(ProjJournalTrans)): return new APMStageHandler_ProjJournalTrans(_common); case (tableNum(InventJournalTrans)): return new APMStageHandler_InventJournalTrans(_common); case (tableNum(APMJobRegistrationJournalTrans)): return new APMStageHandler_JobRegistration(_common); default: throw error(strfmt("@APM1130", tableid2name(_common.TableId))); } }Anonymous
September 12, 2007
Every so often I find myself in a heated debate on what the quality bar should be for the code in theAnonymous
September 12, 2007
Every so often I find myself in a heated debate on what the quality bar should be for the code in the