Standardize Map Stream Argument Names

[Brett208]

Perhaps this is starting to approach a point of irrelevancy on the refactoring. I am trying to standardize the use of streamReader/Writer in names of function arguments.

-Brett

[Brett208]

There is no guarantee that a passed stream is a stream that only contains a map (or saved game). If that doesn't bother you, then I'll change them to read savedGameStream or mapStream.

Thanks, Brett

[DanRStevens]

Good point. I suppose the two file formats are separate, even though they share many sections in common. I'm fine with either of those two suggested names, or even just stream if you want to keep things generic.

It's more naming the variable after the type that I want to avoid.

I suppose the name "stream" does sound like a type, though I would argue that's more of a concept than a type. In particular, we don't actually have a class with the name "Stream". We only use "Stream" as the folder and namespace name.

[Brett208]

Okay, I see your point that we don't want to keep changing the argument name whenever we switch types.

I was actually referring to the fact that you could pass a larger stream into Map::Read that just happened to be currently pointing to a map. Sort of like what we do with an archive file, but not creating a slice first. Although I like the point you brought up (which might actually be a better argument anyways?).

What do you think of:

• Name the public Read/Write Map functions as mapStream
• Name the public Read/Write save functions as savedGameStream
• Name the shared private functions as stream

The user will know the context of whether it is a stream reader or stream writer based on the function name and how stream is called. IE stream.Write inside of a function call WriteTile is clearly some form of streamWriter.

If this sounds good, I'll implement in a new branch and delete this to reduce the noise of commits into the repo.

Thanks, Brett

[DanRStevens]

Sounds like we're on the same page.

I like the names and contexts you've suggested. Let's do that.

I'd suggest making the changes on top of this branch. That way this conversation is better linked to the change history. I'm not too worried about change noise, since the merge commits will isolate much of those details anyway.

[DanRStevens]

Just a small note about the AppVeyor test. One of the builds was cancelled because this pull request was closed before the test completed. The post-merge build did however succeed with all tests passing.