OutpostUniverse / OP2Utility

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

Refactor And Test XFile::ReplaceFilename #233

Closed Brett208 closed 5 years ago

Brett208 commented 5 years ago

Annoyingly enough, on MSVC, it produces:

Expected: XFile::ReplaceFilename("Old.map", "New.map") Which is: "Old.map/New.map" To be equal to: "New.map"

Expected: XFile::ReplaceFilename("Old Space.map", "New Space.map") Which is: "Old Space.map/New Space.map" To be equal to: "New Space.map"

So basically they decided that if you call replace_filename on a filename without a path, what you really meant was append the new filename to the original filename as if it were a directory. How was that a rational choice?

Originally I just wanted to wrap the standard library replace_filename command because it sounded handy. I think that I favor a bit of a heavier standard library than maybe you do. Pros and cons to both. Besides being a smaller standard library than what Microsoft put out for dotnet, I also find the C/C++ library much more difficult to comprehend without pouring over documentation. Of course part of that is caused by how long the language has been around with backward compatibility that dotnet doesn't have to worry about. And part is probably because we are hopefully better at designing libraries as a whole than we were 30 years ago.

-Brett

DanRStevens commented 5 years ago

That is so horrible. :astonished:

I'm wondering if it's an MSVC bug. I just commented out that if block, and tried it with Clang. All tests continue to pass.


I'm afraid the C++17 additions to C++, such as the filesystem library, don't get the excuse of 30 years of backwards compatibility. At best I'd chalk this one up as a bug. Though yes, the long legacy of backwards compatibility does make the older core of the standard library quite a bit of a mess, and often hard to understand. I fully support the idea that people have gotten better at designing libraries than 30 years ago.