Redundancies (and lack of type-safety) in ConstraintEngine::ChangeType and DomainListener::ChangeType #166
Comments
Closing a singleton base domain now marks the variable as specified. This seems inconsistent with a previous commit to distinguish specified variables from variables with singleton base domains, but that commit didn't touch Variable::handleRestrictBaseDomain, which does specify variables whose base domain gets restricted to a singleton. This should be clarified and made consistent. See issue #165. Made the beginning of the ConstraintEngine::ChangeType enumeration and the DomainListener::ChangeType enumeration into line. See issue #166 Added initializers to the EquivalenceClassCollection constructor, which caused some random test failures sometimes.
|
As a point of information, DomainListener::ChangeType is used in DynamicEuropa/ActivityEngine/ReftimeProfile.cc which is in a separate repository from Europa. So if only one is kept, Paul On 12/04/14 11:28 AM, miatauro-NASA wrote:
|
|
It definitely should stay. I suspect the right thing to do is to keep them both, but remove the redundant entries from ConstraintEngine::ChangeType and let the ConstraintEngine re-publish DomainListener::ChangeType messages. |
Closing a singleton base domain now marks the variable as specified. This seems inconsistent with a previous commit to distinguish specified variables from variables with singleton base domains, but that commit didn't touch Variable::handleRestrictBaseDomain, which does specify variables whose base domain gets restricted to a singleton. This should be clarified and made consistent. See issue #165. Made the beginning of the ConstraintEngine::ChangeType enumeration and the DomainListener::ChangeType enumeration into line. See issue #166 Added initializers to the EquivalenceClassCollection constructor, which caused some random test failures sometimes.
Closing a singleton base domain now marks the variable as specified. This seems inconsistent with a previous commit to distinguish specified variables from variables with singleton base domains, but that commit didn't touch Variable::handleRestrictBaseDomain, which does specify variables whose base domain gets restricted to a singleton. This should be clarified and made consistent. See issue #165. Made the beginning of the ConstraintEngine::ChangeType enumeration and the DomainListener::ChangeType enumeration into line. See issue #166 Added initializers to the EquivalenceClassCollection constructor, which caused some random test failures sometimes.
Closing a singleton base domain now marks the variable as specified. This seems inconsistent with a previous commit to distinguish specified variables from variables with singleton base domains, but that commit didn't touch Variable::handleRestrictBaseDomain, which does specify variables whose base domain gets restricted to a singleton. This should be clarified and made consistent. See issue nasa#165. Made the beginning of the ConstraintEngine::ChangeType enumeration and the DomainListener::ChangeType enumeration into line. See issue nasa#166 Added initializers to the EquivalenceClassCollection constructor, which caused some random test failures sometimes.
The first several members of the ConstraintEngine::ChangeType and DomainListener::ChangeType are redundant. Further, they must be kept in sync and are inter-convertable, which can lead to very strange bugs. There should be only one enumeration for these values, and possibly a type-safe idiom (or possibly wholesale movement to C++11's safe enums) should be used.
The text was updated successfully, but these errors were encountered: