nasa / europa

216 stars 72 forks source link

Redundancies (and lack of type-safety) in ConstraintEngine::ChangeType and DomainListener::ChangeType #166

Open miatauro-NASA opened 9 years ago

miatauro-NASA commented 9 years ago

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.

phmorris commented 9 years ago

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, maybe this could be the one?

Paul

On 12/04/14 11:28 AM, miatauro-NASA wrote:

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.


Reply to this email directly or view it on GitHub: https://github.com/nasa/europa/issues/166

miatauro-NASA commented 9 years ago

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.