Unless you’ve been working for an ISV there’s a high percentage of probabilities that you’ve never cared about Dynamics Best Practices (BP), or maybe you have. I haven’t worked for an ISV myself but back when I started working with AX I was handed the development BP document (and you can download the old Microsoft Dynamics AX 2009 development best practices white paper PDF from here) and I’ve tried to follow most of them when writing code.
But BPs could be ignored and not implemented without any issue. This is why Microsoft will publish…
Application Checker
Application Checker is a tool that will change that. It will force some rules that our code will have to meet, otherwise the code won’t compile (and maybe won’t even deploy to the environments).
We got an advance of it during last MBAS session “X++ programming with quality” by Dave Froslie and Peter Villadsen. Unfortunately the session wasn’t recorded.
App checker is built on BaseX, an XML analysis tool, and powers Socratex which Microsoft uses to track code quality. I don’t know if Socratex will be publicly released and I don’t remember if this was clarified during the session.
The set of rules can be found in Application Checker’s GitHub project and it’s still WIP. I think there’s loooots of things to decide before this goes GA, and I’m a bit worried and afraid of some of the rules 😛
Rule types
There’s different types of rules, some will become errors and other warnings. For example:
ExtensionsWithoutPrefix.xq: this rule will throw an error avoiding your code to compile. It checks if the extension class has a name ending with _Extension and an attribute ExtensionOf. If it has it must have a prefix. E.g.: if we extend the class CustPostInvoice it can’t be named CustPostInvoice_Extension, it needs a prefix like CustPostInvoiceAAS_Extension.
SelectForUpdateAbsent.xq: this rule will throw a warning. When there’s a forUpdate clause in a select statement and no doUpdate, update, delete, doDelete or write is called later it will let us know.
As of today, there’s 21 rules in the GitHub project. You can contribute to the project, and you could enforce your own rules without sending them to the project on your dev boxes, just add them to the local rules folder. I’d create a rule that makes the space after an if/while/for/switch mandatory and throws an error otherwise, but that’s only a bit of my OCD when writing/reading code.
Try it on your code
We can already use Application Checker on our development environments since PU26, I think. We just need to install JRE and BaseX in the dev box and select the check when doing a full build.
Some examples
ComplexityIndentationCombined.xq
This query checks the (wait for it…) cyclomatic complexity of the methods. I’ll try to explain it… Cyclomatic complexity is a metric for software quality, and is the number of independent paths in the code. Depending on the number of ifs, whiles, sitches, etc… the code can have different outcomes through different paths, that’s what complexity calculates.
Taking this as an example, a dumb one but ignore it, just look at the amount of different paths that could happen:
class AASAppCheckerDemo
{
public void complexMethod()
{
int a, b, c, d, e, f, g, h, i;
if (a)
{
if (d)
{
if (d)
{
}
else if (e)
{
}
else if(f)
{
}
}
else if (e)
{
if (d)
{
}
else if (e)
{
}
else if(f)
{
}
}
else if(f)
{
if (d)
{
}
else if (e)
{
}
else if(f)
{
}
}
}
else if (b)
{
if (d)
{
if (d)
{
}
else if (e)
{
}
else if(f)
{
}
}
else if (e)
{
if (d)
{
}
else if (e)
{
}
else if(f)
{
}
}
else if(f)
{
if (d)
{
}
else if (e)
{
}
else if(f)
{
}
}
}
else if(c)
{
if (d)
{
if (d)
{
}
else if (e)
{
}
else if(f)
{
}
}
else if (e)
{
if (d)
{
}
else if (e)
{
}
else if(f)
{
}
}
else if(f)
{
if (d)
{
}
else if (e)
{
}
else if(f)
{
}
}
}
}
}
In App checker the error appears when the complexity is over 30. I’ve used Lizard code complexity analyzer to calculate the complexity of the method below and I’m getting a 49.
The rule also checks for the indentation depth, failing if it’s greater than 2. In the end the purpose of both rules is to try to cut up long/large methods, which will also help in enabling more extension points in different places of our logic, like Microsoft did with Data Provider classes for reports.
BalancedTtsStatement.xq
This one gives me mixed feelings. The rule checks that the ttsbegin and the ttscommit of a method are in the same scope. So the following is not possible:
public void ttsCheck()
{
StagingTable stagingTable;
try
{
while select forupdate stagingTable
where !stagingTable.Processed
{
ttsbegin;
boolean ret = this.doThings();
if (ret)
{
stagingTable.Processed = true;
stagingTable.update();
ttscommit;
}
else
{
ttsabort;
ttsbegin;
stagingTable.Processed = false;
stagingTable.ErrorMsg = 'An error ocurred, see log.';
stagingTable.update();
ttscommit;
}
}
}
catch (Exception::Error)
{
ttsabort;
ttsbegin;
stagingTable.Processed = false;
stagingTable.update();
ttscommit;
}
catch
{
ttsabort;
ttsbegin;
stagingTable.Processed = false;
stagingTable.update();
ttscommit;
}
}
private boolean doThings()
{
return true;
}
Imagine you’ve developed an integration with an external application that writes data to an intermediate table in MSDyn365FO and you process all pending data sequentially. You don’t want to throw an error if something goes wrong because you need the process to continue with the following record, so you ttsabort the wrong line, store the error and continue. If this is not possible… how should we do this? Create a batch that creates a task for each line to process?
Plus, the standard models have plenty of ttscommit inside if statements.
RecursiveMethods.xq
This rule will block the use of recursion on static methods. I don’t get why. Application checker should be a way to better coding practices, not forbidding some patterns. If somebody gets a recursive method to prod and the exit condition isn’t met… hello testing?
Some final thoughts
Will this force developers to code better? I don’t think so, but that’s probably not Application checker’s purpose. For centuries humans have found ways to bypass rules, laws and all kinds of restrictions and this won’t be an exception.
Will it help? Hell yes! But the best way to ensure code quality is promoting the best practices in your team, through internal trainings or code reviews. And even then if someone doesn’t care about clean code will keep on writing terrible code, which might work but won’t be beautiful at all.
Finally, I’m not sure about some rules, like avoiding recursion on static methods or the tts thing. We’ll just have to wait and see which rules make it to the final release and how will Application checker be finally implemented in the MSDyn365FO application lifecycle by blocking (or not) the deployments of code which doesn’t pass all the checks or if it will be included into the build process.
