search

OutpostUniverse / OP2Utility

C++ library for working with Outpost 2 related files and tasks.
MIT License
4 stars 0 forks source link

Wrap all of OP2Utility in a global namespace #237

Closed Brett208 closed 3 years ago

Brett208 commented 5 years ago

As OP2Utility matures, it would be nice to wrap the entire library in a global namespace to help resolve eventual name collisions.

Some discussion as to options and syntax were discussed in issue #139.

I think my preference would be OP2Utility. So it would look like:

OP2Utility::Map
OP2Utility::XFile::GetDirectory
OP2Utility::ArtFile
OP2Utility::BitmapFile

Some best practices are only capitalize all letters of an acronym if it is 2 letters or less. We are breaking this, but I think Op2Utility doesn't really look right. I think the community has adopted OP2 as an acronym strongly enough that we wouldn't want to change to something like O2Utility.

The C++ standard library does not use UpperCamelCase and uses underscores between words as well. Since we are using UpperCamelCase for functions and no underscores, I think formatting the namespace name as OP2Utility makes more sense then op2utility. This would make it look like op2utility::MapHeader or op2Utility::ResourceManager. (However all lowercase doesn't really look bad either, if @DanRStevens has a strong preference for op2utility, then I can just live with it). I would like to avoid any underscores though.

In practice, I would probably just declare using OP2Utility at the top of .cpp files of applications using OP2Utility. If a name collision occurred, I would then fall back on full specification.

As a parallel effort, I would be willing to remove the namespace Archives from the code base. Once we wrap the whole library in OP2Utility, then perhaps there isn't enough Archive specific code to justify a sub-namespace.

For now, I'm happy leaving the Stream namespace. So you would have OP2Utility::Stream::Reader. Again, I would probably call using on OP2Utility, so in practice you would see Stream::Reader.

Thanks, Brett

DanRStevens commented 5 years ago

As discussed in Issue #139, "OP2" is not really an acronym, but rather an abbreviation, and so should be avoided as per naming recommendations. However, it is long established practice at OPU (which for the same reasons is also not an acronym). I think we should stick with "OP2", and use "OP2Utility" for the namespace.

And besides, "OP2" is technically only 2 letters, and so should be capitalized. :wink:

Additionally, I want to add that we should stick with the casing conventions used by code from Outpost2.exe, which matches the Windows convention of upper camel case, and no underscores. Underscores are horrible.

I'm undecided about the Archives namespace. (Should namespaces be pluralized?) I'd actually kind of forgot we even had it.

I think the new namespace should encapsulate everything. If we want to exclude Stream from the namespace, we should do that by splitting the project. That's not something I really want to do at this time.

I suppose along the lines of avoiding abbreviations, is there perhaps an alternate library name we could think of that doesn't use "OP2", yet still implies this library targets being compatible with Outpost 2 resources?

Brett208 commented 5 years ago

Okay, it seems we agree on OP2Utility. Once we clear out the current pull requests, I'll implement. It will go smoother if it isn't in the middle of other work.

If you are on the fence about Archives, we can just leave it for now. I'm happy with either Archive or Archives. Since we have Stream, it would probably be more consistent to just use Archive. If we want to change it to Archive, probably best at the same time as wrapping everything in OP2Utility to minimize large codebase changes.

For some context, in C# projects I use and make tend to create a namespace for each directory in the source code. So in this case, we would have a namespace for map, archive, bitmap, stream, etc. Since you don't have to #include everything, the using statements are what section off the code for you and keep the list of items picked up by Intellisence smaller. By staying close to the directory structure, you can sort of tell where the source code file will be located based on the using statement. That is why I made the Archives namespace which matches the Archives directory. Of course C++ is a bit different with both using and including going on so I don't know if that is a best practice or not.

I cannot think of a different name besides saying Outpost2Utility outright which I don't think is necessary? We could say DynamixRtsUtility or SierraHardSciFiPostApocolypseUnderRatedGameUtility...

DanRStevens commented 5 years ago

Ohh, ohh, ohh, SierraHardSciFiPostApocolypseUnderRatedGameUtility! I like it! Let's go with that. :rofl:

I suppose the folder could be renamed to just Archive. You're right about the consistency thing.

I like the nested namespace structure of C#, though I'm afraid it doesn't work quite so well in C++ as the nesting gets deep. The header file structure of C++ means you want to avoid putting using in a header file, and so deeply nested structures quickly become really obnoxious.