STYLE: Clean-up ByteSwapper, using private SwapBytes helper function

[N-Dekker]

Aims to reduce code duplication. Inspired by a suggestion from Bradley (@blowekamp) at https://github.com/InsightSoftwareConsortium/ITK/pull/4841/files#r1751777344

[blowekamp]

This is a run-time exception for a "constexpr" conditional?

[N-Dekker]

Indeed! Only the condition of an if constexpr (condition) { ... } must be evaluated at compile-time. Anything inside the if or else clause may be evaluated at run-time.

[N-Dekker]

Instead of this exception, we might also just trigger a compile-time assert failure (static_assert). But obviously, that would change the behavior of ByteSwapper (at least for an unsupported number of bytes). It might be considered an improvement, to replace the exception with a static_assert, of course. Would you like that?

We might just do:

static_assert(numberOfBytes == 2 || numberOfBytes == 4 || numberOfBytes == 8);

Would you like that?

[blowekamp]

Yes. I think it would be an improvement and help detect errors in code.

[N-Dekker]

Honestly it would be fine to me to also support other number of bytes. Why not support numberOfBytes == 16, for example? For example, when a user wants to swap 128-bit integers...!

I think the original itkGenericExceptionMacro("Cannot swap number of bytes requested") was there just because we only had three of those helper functions: Swap2, Swap4, and Swap8. Instead, we can now support just any number of bytes! Would you like that?

[N-Dekker]

Still broken, now it's itkVTKPolyDataMeshIO, at https://open.cdash.org/viewBuildError.php?type=1&buildid=9915657:

in instantiation of function template specialization 'itk::VTKPolyDataMeshIO::ReadPointsBufferAsBINARY' requested here

  CASE_INVOKE_BY_TYPE(ReadPointsBufferAsBINARY, inputFile)
                      ^

So would you also want us to drop long double support from itk::VTKPolyDataMeshIO?

[dzenanz]

Unsurprisingly, this endian swapping only affects IO classes.

As the other float types are being swapped, why not swap long double as well? Just add size of the long double to the list of the allowed sizes in the assert? @blowekamp what do you think?

[blowekamp]

I quick google said 128-bit float may be an abnormal case:
https://www.talospace.com/2018/12/the-saga-of-power-isa-128-bit-long.html

So... Just leave it as a run time exception of now?

[N-Dekker]

It would be fine to me to restore the itkGenericExceptionMacro("Cannot swap number of bytes requested") that I originally proposed, and remove the compile-time static_assert(numberOfBytes is 2 or 4 or 8).

[dzenanz]

That should leave support for long double if there is no need for endian swapping (the usual case).

[blowekamp]

Looking like a nice cleanup!

[dzenanz]

Good even as-is.