Download Reference Manual
The Developer's Library for D
About Wiki Forums Source Search Contact

Ticket #847 (closed enhancement: fixed)

Opened 16 years ago

Last modified 16 years ago

ByteSwap usability

Reported by: mwarning Assigned to: kris
Priority: normal Milestone: 0.99.5
Component: Documentation Version: 0.99.4 Frank
Keywords: Cc:

Description

core.ByteSwap? lacks some usability/documentation. Some thoughts:

- Some examples would be nice:

ubyte[] x = [0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08];

swap16(&x[0], 8) -> 02 01 04 03 06 05 08 07

swap32(&x[0], 8) -> 04 03 02 01 08 07 06 05

swap64(&x[0], 8) -> 08 07 06 05 04 03 02 01

"bytes" (second argument) must be a multiple of 2 for swap16 (4 for swap32, 8 for swap64) and also bytes shouldn't be > length of data void* dst points to. In other cases, it will segfault.

- Therefore asserts would be nice:

assert(bytes%2 == 0, "bytes must be multiple of 2");

for swap16 and so on.

- Also should the documentation point out that bytes is the lenght of the data structure void* dst points to.

- Also nice to have:

swap(ushort i) { return swap16(&i, ushort.sizeof); }
swap(uint i) { return swap32(&i, uint.sizeof); }
swap(ulong i) { return swap64(&i, ulong.sizeof); }

.. it just looks better to use and is fool-proof, and probably what most people want to do with ByteSwap?.

- Maybe also replace swap16(void*, uint) by swap16(void[]). It would reduce possible errors (assert for wrong array size can be added). Or are there specific reasons for using the latter one?

Btw.: ByteSwap? is indeed faster than byte shifts, ~22% on my system Athlon XP 2000.

Change History

01/06/08 16:40:05 changed by larsivi

  • owner changed from kris to sean.
  • milestone set to 0.99.5.

01/07/08 05:46:55 changed by kris

  • owner changed from sean to kris.

02/11/08 02:41:43 changed by kris

  • status changed from new to closed.
  • resolution set to fixed.

fixed in [3161]

Thanks, mandel

02/17/08 02:07:08 changed by mandel

  • status changed from closed to reopened.
  • resolution deleted.

The added text for swap16 says:

/***********************************************************************

                Reverses two-byte sequences. Parameter bytes specifies the 
                number of bytes, which should be a multiple of 2

 ***********************************************************************/

        final static void swap16 (void[] dst)

- There is no mentioned parameter bytes. What about this text?:

Reverses every two-byte pair.
Array length must be a multiple of 2.

- The hex lines for the example array output are displayed in one line in the online api documentation. Please add an extra line in between or smth. like that.

- please also replace "->" by "=>"; it's a bit nicer, more common use in Tango.

- what about ushort swap(ushort)/uint swap(uint)/.. to reverse the byte order?

  • it should have no speed difference for basic types
  • fit's nicer into the code, no error because of wrong bytes parameter possible.

The drawback is that ByteSwap? may look crowed. But maybe we could remove the swapXY(void* dst, uint bytes) versions at some point, since we could also use swapXY(void[] dst) calling it this way: swapXY(dst[0..bytes]); .

02/17/08 02:21:50 changed by kris

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [3209]) closes #847