bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: A new module: bitset


From: Bruno Haible
Subject: Re: A new module: bitset
Date: Sun, 25 Nov 2018 16:44:33 +0100
User-agent: KMail/5.1.3 (Linux/4.4.0-138-generic; KDE/5.18.0; x86_64; ; )

Hi Akim,

This code looks like well-designed, high-quality and very suitable for
gnulib.

I love especially about it that it's adaptable as programs change:
Initially the program might require fixed-size bitsets, and later
move to variable-size bitsets.

> I think it should move to gnulib

Yes, definitely.

> I have written a short
> doc so that the reader can get at least a sense of how to use this
> library.

This doc is good and essential (to get an overview).

Some remarks:

* I can imagine programs that want bitsets but no bitset-vectors.
  Suggestion: Move the bitsetv* code to a second module, named
  'bitset-vector'.

* File names are nowadays not limited to 8+3 characters, and names
  like 'abitset.h', 'bbitset.h', 'ebitset.h', 'lbitset.h', 'vbitset.h'
  are not really telling. How about using more descriptive names?
  Note: You can use subdirectories to keep files grouped together,
  especially since the only file the user sees is bitset.h.
  How about renaming e.g.
    abitset.[hc] -> bitset/array.[hc]
    lbitset.[hc] -> bitset/list.[hc]
  and keeping only bitset.h at the top level?

* In the tests, the function
    void check_attributes (enum bitset_attr attr)
  ignores its attr argument. Probably you meant to use attr instead of
  BITSET_FIXED?

* Regarding tests: There is a large amount of algorithmic code. While
  the parts that Bison uses are surely correct, there is a possibility
  of bugs in the more rarely used parts. I would find it very useful
  to have unit tests of all 32 operations.

  An approach that minimizes the code to be written would be to
  do the same operations on, say, list-bitsets and array-bitsets, and
  compare the results. This way, you can infer the correctness of the
  list-bitsets implementation, assuming the correctness of the array-bitsets.
  (This approach is already used in the test-*list.c tests.)

* The hint BITSET_FRUGAL is unused. How about eliminating it?

Bruno




reply via email to

[Prev in Thread] Current Thread [Next in Thread]