[Feature Request]: Make BitInFormat copyable and movable

[josefschroettle]

Feature description

There is no real reason why BitInFormat is not copyable&movable, it just contains an uchar. The const char* in BitOutFormat is just a copy of the passed pointer (and not allocated memory) so BitOutFormat can also be made copyable and moveable.
Making them copyable would allow logic to determine the format from the extension, e.g.
if (ext == ".rar") format = bit7z::BitFormat::Rar
and later pass the format to bit7z::BitArchiveReader
Note: bit7z::BitFormat::Auto is currently not working for ".7z.001" and ".zip.001" as created by 7z so the workaround to set the format by the 'relevant extension' is needed...

Additional context

No response

Code of Conduct

• By submitting this issue, I agree to follow bit7z's Code of Conduct

[rikyoz]

There is no real reason why BitInFormat is not copyable&movable, it just contains an uchar. The const char* in BitOutFormat is just a copy of the passed pointer (and not allocated memory) so BitOutFormat can also be made copyable and moveable.

If I remember correctly (it was a decision made a long time ago, in 2018 according to git), I made these classes neither copyable nor movable to enforce reference semantics when using/passing them.
The idea was that the user should only use bit7z's constants in the BitFormat namespace, preventing copies/moves.
Also, BitInFormat is cheap to copy, but the size of BitInOutFormat is currently above the usual 2 * sizeof(void*) threshold for when passing by value is cheap (at least on 64-bit platforms), so it's better to pass it by reference.

In my defence, I was much less experienced with C++ and concepts like value semantics at the time. If I were to design these classes from scratch now, I would probably make them copyable/movable.

I think I can make them so in the next v4.1-beta, although it will require some layout changes in BitInOutFormat. I'll have to test whether such a change would cause any unexpected problems and maintain backwards compatibility (it should).

[josefschroettle]

Yes, as an argument to a function it can make sense. But forcing users into reference semantics is not a good idea. Example: Once in a while I copy std::string by value if I have to change it in the function right away. If I pass the string by const reference and make a copy in the function the effect is almost the same, but then I have two names (one for the parameter and one for the local variable) which reduces readability. There are many cases where a copy is plain wrong, e.g. if the class contains a raw pointer which is allocated in the class, or the class contains a mutex, etc. But forcing users into workarounds just because of an assumed performance penalty is not a good reason to prevent a copy. And there is almost no reason to prevent moves...

[rikyoz]

Yeah, I completely agree with everything you said.
I follow the same principles as you now, but as I said, I was much less experienced with C++ back then.
At the time, I simply thought it was a good idea to enforce reference semantics so that only my constants could be used.
I was wrong :)