Looking at changes in the 3.0 branch to cob_file_key in
libcob/common.h (from 2.0), I see a move to discrete flags for whether
the key allows duplicates, is ascending, and if missing values are
suppressed. While I applaud the intention, I would like to suggest a
more efficient scheme that would be easier to use.
The current struct looks, in part, like this:
typedef struct __cob_file_key {
...
int tf_duplicates;
int tf_ascending;
int tf_suppress;
...
} cob_file_key;
That's 3 * 8 = 24 bytes on a 64-bit architecture for three boolean
values, when only 3 bits are needed.
I suggest instead enumerated values for the flags, and functions to
test and set them.
enum cob_file_option_t {
CF_DUPLICATES = 1,
CF_ASCENDING = 2,
CF_SUPPRESS = 4,
CF_ALL_FLAGS = 7
};
typedef struct __cob_file_key {
...
int options;
...
} cob_file_key;
static inline bool
duplicate_keys( const cob_file_key *key ) {
return (key->options & CF_DUPLICATES) > 0;
}
static inline bool
ascending_keys( const cob_file_key *key ) {
return (key->options & CF_ASCENDING) > 0;
}
static inline bool
suppressed_keys( const cob_file_key *key ) {
return (key->options & CF_SUPPRESS) > 0;
}
static inline int
cob_file_key_option_set( cob_file_key *key,
enum cob_file_option_t option )
{
return (key->options |= option);
}
static inline int
cob_file_key_option_clear( cob_file_key *key,
enum cob_file_option_t option )
{
return (key->options &= ~option);
}
Here I'm taking advantage of a 20-year-old standard, C99, which
introduced static inline functions and stdbool.h.
IMO the code becomes clearer by use of the functions. For example:
if( key->tf_duplicates == 0 )
versus
if( !duplicate_keys(key) )
The places where the key is being set/reset is made easier to locate in
the code by the functions. Instead of looking for a variable
assignment, we look for function invocation:
key->tf_duplicates = 1;
versus
cob_file_key_option_set( key, CF_DUPLICATES);
If you feel these names are too long, I might agree. I could also be
happy with cfk_set and cfk_clear if those were deemed acceptable. In
fact, if it's not too fancy, I'd suggest PROP_SET and PROP_CLEAR macros,
and use _Generic (from C11) to discriminate underlying functions by
argument type (a bit like function overloading).
I would like to switch to this system of key properties as part of our
work on the LMDB ISAM implementation. I wanted to get a sense of the
senate first.
--jkl
