Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Structure serialization to use Dictionary #993

Merged
merged 10 commits into from
Jun 24, 2021

Conversation

DanRStevens
Copy link
Member

@DanRStevens DanRStevens commented Jun 23, 2021

Refactor some of the Structure serialization code to use NAS2D::Dictionary.

I think this gives enough of a structure that people can start to convert serialization code to use NAS2D::Dictionary, and minimize direct reliance on XML code.

Reference: #830


I'm sure there's more to do with Structure serialization. In particular, the various sub-classes can override the getDataDict() method to add in their structure specific fields. That would allow much of the custom processing in the higher level serializeStructure function to be eliminated. The Factory class has been handled as an example.

Not addressed are the use of sub-elements to store additional structure data. For example, the <food level="0" /> sub-element was not changed at all. The Dictionary object doesn't really provide a multi-level structure to the data, so that concept doesn't map well to Dictionary. If there is a fixed maximum nesting, such as at most 2 levels, we could instead use a std::map of Dictionary or similar. Though I suspect some of the current nesting can be removed, making the point perhaps a bit irrelevant.

In terms of reducing the nesting level, some sub-element values can be moved to regular attributes, such as food="0". Currently there is some inconsistency in the code as to what format to use. Some sub-classes attach additional attributes to the XmlElement, others add child XmlElement objects for the data. I suggest we try to improve consistency by using attributes on the main structure tag whenever possible.

Example (existing format):

<structure x="60" y="98" depth="0" ... pop0="0" pop1="0">
    <food level="0" />
</structure>

Suggested:

<structure x="60" y="98" depth="0" ... pop0="0" pop1="0" food="0" />

@ldicker83
Copy link
Collaborator

This looks pretty good. The only recommendation I have is to spell out dictionary versus dict. I wouldn't consider this blocking so I'm not going to make it a requirement and instead leave it up to your judgement.

That said, this simplifies things a lot and makes it a hell of a lot easier to read. It also helps to decouple it from an XML implementation so it could be much more easily changed to something else if deemed fit in the future.

@DanRStevens DanRStevens merged commit e51666a into master Jun 24, 2021
@DanRStevens DanRStevens deleted the refactorSerialization branch June 24, 2021 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants