Confusing naming for identifiers

[ngjunsiang]

Identifier naming

Identifier classes

There is currently an EntityID class:

open-dis-python/opendis/dis7.py

Lines 3477 to 3490 in 3a52c4f

	class EntityID( object ):
	"""more laconically named EntityIdentifier"""
	def __init__(self,
	siteID=0,
	applicationID=0,
	entityID=0):
	""" Initializer for EntityID"""
	self.siteID = siteID
	""" Site ID"""
	self.applicationID = applicationID
	""" application number ID"""
	self.entityID = entityID
	""" Entity number ID"""

But there is also an EntityIdentifier class:

open-dis-python/opendis/dis7.py

Lines 3421 to 3431 in 3a52c4f

	class EntityIdentifier( object ):
	"""Entity Identifier. Unique ID for entities in the world. Consists of an simulation address and a entity number. Section 6.2.28."""
	def __init__(self,
	simulationAddress=None,
	entityNumber=0):
	""" Initializer for EntityIdentifier"""
	self.simulationAddress = simulationAddress or SimulationAddress()
	""" Site and application IDs"""
	self.entityNumber = entityNumber
	""" Entity number"""

There are also other Identifier classes:

• IntercomIdentifier
• EventIdentifier
• ObjectIdentifier
• SystemIdentifier
• AggregateIdentifier
• RadioIdentifier
• UnattachedIdentifier
• EventIdentifierLiveEntity
• LiveEntityIdentifier
• MineEntityIdentifier
• SimulationIdentifier
• MinefieldIdentifier

EntityID seems to stick out to me here as being named differently from the rest, yet it is used throughout dis7.py.

Identifier reference

Section 4.2.5.2 refers to object identifiers as "the designators assigned to uniquely identify objects such as entities, aggregates, minefields, environmental processes, groups of entities and points, and linear and areal objects."

Section 4.2.5.3 refers to other identifiers as being "used to designate other data items that are not objects."

Proposal

For closer coherence with the spec and documentation, and to lay some groundwork for future features, I propose the following changes.

Rename EntityID

To reduce dissonance between the codebase and the documentation, I propose to use EntityIdentifier as the standard identifier class for entities. All object identifiers should also have their init parameters take a siteNo, appNo, and refNo (instead of a SimulationAddress and a refNo).

Make ObjectIdentifier a superclass

To make instance/subclass checking (using isinstance() and issubclass()) possible, I propose to have the following classes inherit from an ObjectIdentifier superclass:

• EntityIdentifier
• AggregateIdentifier
• EnvObjectIdentifier (renamed from ObjectIdentifier above; shared by PointObjectIdentifier, Linear Object Identifier, and Areal Object Identifier)
• LiveEntityIdentifier
• GroupIdentifier
• GroupEntityIdentifier
• MinefieldIdentifier
• MineEntityIdentifier
• EnvProcessIdentifier (or spelled out in full)

This would be useful for future work, since ObjectIdentifiers are supposed to be unique even across object types. It would enable tracking of instantiated ObjectIdentifier subclasses through the superclass, if such a feature is desired.

Other Identifiers

These identifiers, due to their varied nature and interface, should remain independent classes that do not subclass ObjectIdentifier. They will nonetheless still retain the -Identifier suffix.

Namespace identifier classes separately

Many attributes in dis7.py are named/suffixed as ID, yet do not hold an instance of one of the above classes (e.g. actionID in ActionRequestPdu is an enum struct). Separating the object identifiers (4.2.5.2) and other identifiers (4.2.5.3) more distinctly from the other classes will make reference lookup easier and help with disambiguation.

I suggest moving them to an identifier.py file, since none of the values rely on other classes in dis7.py, aside from SystemIdentifier requiring an instance of ChangeOptions which no other class relies on. The classes can be explicitly imported into dis7.py by name, so no other changes to dis7.py are needed.

I'm willing to make a pull request incorporating these changes.