Enhancement suggestion for std.bitmanip

[H. S. Teoh]

On Fri, Aug 09, 2013 at 01:03:15PM +0200, Baz wrote:
> I'd like to suggest some stuff to add to bitmanip: the concept of
> set of enums elements. The idea is simple: the enum members are
> stored in a set according to their rank.
>
> So instead of OR-ing some consecutive values as usually done in C or
> C++ it's possible to do the same with the complex enums (based on a
> floating point type for example, or with some non-consecutives
> integer values).

Nice idea, I like it!


> I've written a module which demonstrates the concept. Tell me your
> thoughts about this idea. By the way, I'm a relatively a new comer
> in D, I've read on the wiki that there is a procedure to follow in
> to propose a new phobos module, but in this case I'd just like to
> propose some additions, not a new module, so be tolerant if I don't
> behave like expected...
> 
> The module which illustrates what I'd like to be added:
> https://github.com/BBasile/BitSets
> 
> Rationale: The best argument I can give is that this is inspired by
> a Delphi language feature (the Sets) widely used. It keeps a logical
> relationship between the members of an enum in a single container
> (instead of having many fields related to the same context, it uses
> a single one). That's also very type-safe because it's based upon a
> template, the traits and a parametric struct.

I think the idea is sound. But I think the implementation can be
improved:

- It seems that right now we are limited to at most 64 elements in the
  enum. This is probably not a big deal (I can't see any *good* design
  requiring an option set with more than 64 items), but it would be nice
  if it was somehow possible to implement it for larger enums too.

- I'm not sure if you were planning to do this later, but the ddocs in
  the current code needs some improvement. For example, just by reading
  the ddoc comments, I can't figure out how to use the bitsets. I have
  to read the actual implementation to figure this out.  If you're
  hoping to get this into Phobos, you'll need to provide enough code
  examples in the ddocs.

- sSet.this(): I don't think it's a good idea to initialize the set
  using a runtime string, especially in a ctor. That would be too
  inefficient. It's probably better to provide a method for doing the
  conversion (I can't remember offhand how Phobos does it; ideally,
  you'd want it to work seamlessly with std.conv.to).

  Also, it's a bad idea to just return from the ctor when the string
  representation doesn't match the expected syntax. You should throw an
  exception instead.

- toString() can be made more efficient by using this overload instead
  of the one that returns a string:

	void toString(scope void delegate(const(char)[] s) put) {
		put("[");
		foreach (elem; ...) {
			...
			formatValue(put, elem, FormatSpec!char("%s"));
		}
		put("]");
	}

  This avoids as many string allocations as possible.

- Method names should be camelCased, not Capitalized.

- The way you have it set up right now requires rather verbose syntax to
  use. It would be nice if you supported convenient syntax for inline
  set declarations, because the most usual use cases for your bitsets
  are to pass options to library functions. So in the ideal case, you'd
  want to be able to use syntax like:

	enum MazeOpt { AllowLoops, MultiExits, AddTrapDoors };
  	void makeMaze(int width, int height, sSet!MazeOption options) { ... }
	...
	void main() {
		// inline declaration of options set
		makeMaze(10, 10, sSet!(MazeOpt.MultiExits));

		// allow multiple options to be specified; I used a
		// template here because ideally, you want this to
		// translate directly into the combined binary value so
		// that you aren't computing the set at runtime.
		makeMaze(10, 10, sSet!(MazeOpt.AllowLoops, MazeOpt.MultiExits));

		auto opts = sSet!(MazeOpt.MultiExits);
		auto opts2 = sSet!(MazeOpt.AddTrapDoors);
		makeMaze(10, 10, opts | opts2); // overload "|" to work between sets in a typesafe way
	}

  I'm not 100% sure about using "|", because then there's no convenient
  way of expressing exclusion; maybe a better way is to overload + and
  -, then you could write opts + opts2 to combine two sets of options,
  or opts - opts2 to exclude the second set from the first.


T

-- 
A bend in the road is not the end of the road unless you fail to make the turn. -- Brian White