Version checking when reading deeply nested structures

Question

I need to open a game save file, read it, maybe modify it and then save it.

The structure of the save is quite simple: it is an object with some members, some of which are data, while others are other objects, those objects have some members, some of which are data, while others are other objects, you see where this is going...

Since the game is still in development and the save format is ever changing, the game devs have added a version member as an int to any objects that are being changed.

I have some code that I've written, but it's embedded in my program, and I want to take it out and create a dll that will, when given a stream, try to read it, return the save object if everything is alright, or return an error if not... ...somehow.

Now, the flow is quite simple:

1. Open an object
2. Check if the version matches
3. If it does, continue reading until done, either members or objects (in which case goto 1)
4. If not, note the information (expected and read version, class name, I do have them available at that point) and stop any further reading of the file. Just return an error.

The way I have it currently setup is with macros. The reason for this is that I have to check for version over 40 times, and I really don't want to duplicate the same logic that revolves around this, more so that a single version check result will need to be evaluated on each layer until it reaches the root of the object.

The macro gets the expected and current version and then if everything is okay, it will just continue the program, but if something is awry, it will log the name of the class and return from the function it was in, returning the current version, while the function would otherwise continue and return 0. Like this:

#define CHECK_VERSION(currentValue, expectedValue) {int versionCheck = checkVersion(currentValue, expectedValue); if (versionCheck != 0) return versionCheck;}

//checkVersion() is a function that returns 0 if all is good or, if not, logs it to file and returns the currentValue)

int SomeObject::read(...) {
  int objectVersion;
  someReader::readInt(&objectVersion);
  CHECK_VERSION(objectVersion, expectedVersion);
  //read other stuff
  return 0;
}

Of course, that means that every time an object reads another object, it needs to check if the return value is 0 (if so okay) or not (if so, return that value immediately) which is another macro (and which is repeated on each layer of the structure, to see if any internal objects have failed to be read)

In the end, this makes for a convoluted system that does work, but I am now unsure of how to separate it into a dll, nor think it's the best solution.

The main problem here is that I want create a way for end user to, when it tries to read a save either gets the save, or in case of failure gets the name of the object that failed along with the version number that caused the mismatch.

I can't just write it to file, I need to somehow get that information to the top level and let the user to decide how to handle the problem and whether to log anything or not.

So the question is, how do I structure this? When I check the version of an object and it fails, what do I do? Try-catch with my own exception that would propagate to the root object seems like a terrible idea, given how deep the stack could get.

Should I do classic error codes and create an object that contains error data and just return that upwards, similar to how I've been returning the single number? I would still need to use macros to return from the function early, which will again make it ugly, or force me to repeat a lot of code which again is no good. But this is the only option that I can think of right now.

Is there a better way to handle this that would be more elegant and would not require the use of macros while keeping the code duplication to the minimum?

If you want to check out my exact code, you can see it here. The link is to the relevant commit, if someone stumbles upon this in the future. The classes you want to look at are "EntityCreationData" and "VersionCheck". The problem appears in many more, but those should paint the picture well enough.

Answer 1

Exceptions are an excellent tools when the location where you detect an error is far removed (in terms of stack depth) from the place where you can sensibly handle the error.

You could write your macro as

#define CHECK_VERSION(currentValue, expectedValue) if (checkVersion(currentValue, expectedValue) != 0) throw VersionMismatch(currentValue, expectedValue)

and write a try {...} catch(VersionMismatch) { handle version mismatch } around the top-level parsing function.

The exception handling mechanism of C++ will ensure for you that all intermediate processing is aborted and all stack frames are cleaned up for you (including calling all needed destructors).

Answer 2

If I understand correctly, you should neither return error codes nor throw exceptions. Instead, return an object that contains parsing status and metadata (object type, version) and the decoded content.

Use this object to rebuild the object tree in your own code.'Color' nodes according to their parsing status. All white nodes could be parsed successfully, while black nodes have an unknown/illegal version number and don't allow (I assume) any further travel down the tree.

Most likely, you can still provide partial functionality only using the white node while not touching the black nodes.

I would suggest this approach since the data structure changes so quickly and errors are to be expected. Therefore, you can build this assumption into your architecture to make everything easier for you and your users.