[GnuCOBOL-users] flags in cob_file_key

[James K. Lowden]

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