|Author||[OPEN] $assert improvement|
AT core team member
|19 April 2013 16:52|
The purpose of the discussion is to improve the way $assert is managed.
The current signature, $assert : function (id, value), take an id to easily retrieve the error location.
The result is that we put a lot of $assert with number, like "this.$assert(284, this._mdp != null);".
The issues with this implementation are :
- The numbers doesn't help much in finding the location, as a simple breakpoint do the trick,
- By the way, they add an useless code, increasing the files size (even if it's not much off course, but still useless),
- They are hard to maintain :
- if we want them unique, it's hard to find what is the next one,
- If we want them to be related to the line number, this information will be unrevelant with the future file changes.
So, if it's useless, why not just removing them ?
Moreover, the "expected" assert pattern is not completly followed. I mean, if an assert is used in the code, my expectation is that all the following code isn't worth to be run, as I havn't my requirement (the assert test). So the assert function should throw an exception instead of just logging an error.
The new signature could be :
- test : the value to be tested,
- errorMsg : either a string or a json with a string and some parameters, to be used with a this.$logError
- throwException : true/false, to throw or not an exception is the test fails. The default value has to be defined, but in my opinion, it should be true, as an assert is assumed to be always true !
To be suitable to every case, the return value will be true or false (usefull if throwException = false).
The other advatage of this new signature is to reduce the code size, avoiding these kind of test :
which would be replaced by (here the code size change isn't revelant, the second case is more interesting) :
Or if we want to assume that myTest should absolutely be true :
Also note that the error messages is usefull only if myTest has to be provided by external users, an internal check doesn't require it.
(This post was last modified: 27 November 2013 14:10 by susanta.behera.)