-
Notifications
You must be signed in to change notification settings - Fork 41
Guide: coding with automated error detection
One of the issues when programming is to figure out what to do with unexpected contents of variables, like what if the code tries to read index -1 in an array.
When considering invalid values of variables and what to do about it, the first question is if it's possible that an invalid value will appear due to a bug or if it might happen even if the code is bug free.
We assume the DLL always provide the correct data to other parts of the code. According to the dictionary, there is a word for blindly trust something to be the case without any evidence.
assert, declare, affirm, protest, avow mean to state positively usually in anticipation of denial or objection. assert implies stating confidently without need for proof or regard for evidence.
In other words we assert a lot about arguments and return values of functions. If we tell the compiler what we assert, the compiler can help us figure out if our assertions are true.
There are 3 functions to use for writing assertions.
This is the simple approach. Say you get a pointer to a unit as argument, then you assert the pointer to not be NULL. That would be written:
FAssert(pUnit != NULL);
This will then trigger an assert failure if the pointer is in fact NULL. It will display a popup window telling which line failed and that's about it.
It's an addon to FAssert. If it fails, it will add the string to the popup window.
FAssert(pUnit != NULL, "pUnit is not allowed to be NULL");
You can generate the string at runtime using
CvString::format("").c_str()
This gives a printf interface to generating the string. Combined it looks like this:
FAssert(pUnit != NULL, CvString::format("Unit %d provided a NULL pointer", iUnit).c_str());
While the previous two asserts are for runtime testing, this one is for compile time testing. This means it can't be used to test the content of variables as they aren't known to the compiler. It can however test conditions known at compile time.
// make sure the variables use the same amount of memory
BOOST_STATIC_ASSERT( sizeof(T) == sizeof(int));
// make sure all enum values can fit in a single byte
BOOST_STATIC_ASSERT(NUM_SOME_ENUM_TYPES <= 0x100);
// note: this will ensure everything before NUM_SOME_ENUM_TYPES fits in a byte
// while we ignore NUM_SOME_ENUM_TYPES itself can fit
Usually the runtime asserts will be the most used and it's not surprisingly if somebody never end up with a good use case for BOOST_STATIC_ASSERT.
This is conditions you expect to happen and you write regular C++ code to handle them. An example could be:
if (pPlot == NULL)
{
return 0;
}
return pPlot->getSomeValue();
This can be perfectly valid code. Imagine looping through all the plots next to a unit/city. If the unit/city plot is on the edge of the map, some of the plots will be NULL. That's not a bug, that's by design. No non-bug conditions should trigger an assert failure.
Since this is an edge case and NULL pointers might not be used in multiple games in a row, it's a place where bugs can go unnoticed. This is a strong indication that you shouldn't assume functions to always return valid data even if you frequently use those functions without any issues.
If you for some reason feels like say writing a loop to count all the contents and then assert on the result, the approach is this:
#ifdef FASSERT_ENABLE
// loop
FAssert(result == iArgument);
#endif
The major difference between using regular C++ code and an assert is compiler settings. If the compiler is set to Release or Profile, FAssert and FAssertMsg are ignored. This mean the stable releases aren't slowed down by frequent data verification. On the other hand, the if based checks will slow down release builds.
Say you write a function, which takes eYield as argument. You write it to be called by some code, which loops all yields. It would be natural to add the first line:
FAssert(eYield >= 0 && eYield < NUM_YIELD_TYPES);
Since you wrote the function to only be called by the loop, you know there should be a YieldInfo for each value and you can go ahead and use it. If somebody later reuse your function and calls it with NO_YIELD, it will assert, which is a message to us telling "the programmer told us this won't work. There is something we need to investigate here".
You can expand on this like:
FAssert(eYield >= 0 && eYield < NUM_YIELD_TYPES);
if (eYield < 0 || eYield >= NUM_YIELD_TYPES) return 0;
If it makes at least some sense, it could prevent a crashing bug in a release while it still tells us something is wrong and not working as intended.
If you trust the code, you use asserts. If you don't, then you use runtime checks.
You can trust the DLL in most cases, but you have to judge on a case by case basis. Getting the plots when looping city plots sounds like it can be trusted and if you are unaware of the edge issue, you can end up ignoring it and just use the pointers without checking. If you assert check to see if the pointers aren't NULL, at least it will tell where something is wrong.
The python interface shouldn't use asserts. It's runtime exclusively and can return NULL or other garbage if the arguments are invalid. It's then up to the python code to either handle NULL replies or cause a python error.
Asserts are used when checking xml files. While the DLL can't trust the xml files to be set correctly, we assume the xml modders to use a DLL with enabled asserts. Also the asserts should be written in a way that if it doesn't trigger any asserts prior to the main menu, then the xml setup should be fine. Asserting on xml errors should always use FAssertMsg with well written messages, often telling CvInfoBase::getType() if possible. A poorly written assert message or even just FAssert will require a debugger to tell what went wrong. A well written message will tell what to correct in xml without any further investigation. This is important because we can't assume xml modders to have the skills or even software to debug.