Skip to main content
added 203 characters in body
Source Link
chux
  • 36.5k
  • 2
  • 43
  • 97

This is simple a first pass with mostly top level thoughts - I dig deeper later.

  1. Suggest moving all functions to bst.c including bst_init() size_t bst_encode() bst_decode(). By doing so, ENCODING_SIZE_T MAX_CONTENT_LENGTH BST_Node and the fields of BST may be removed from the application view. The ap does not need to see them.
    [Edit] I now see bst_init() size_t bst_encode() bst_decode() are only prototyped in bst.h - ignore that part of this suggestion. At first blush, the style looked like code, but it is only prototype.

     // BST.h
     typedef struct BST BST;`
    
     // BST.c
     struct BST { 
       BST_Node *root; 
       size_t node_count; 
       int (*compare)(void *, void *);
     };
    
  2. I often use the prefix before functions like BST_This() and BST_That(), but I would be consistent with the filename's case. Use BST.c with BST_This() or bst.c with bst_This().

  3. Need #include <stdlib.h> for size_t in bst.h. You might have caught that if in bst.c you included the standard headers after "bst.h". Suggest moving those after "bst.h". This prevents requiring users of bst.h to include certain headers files first.

  4. Large comment blocks before bst_encode in both bst.h bst.c is a maintenance liability. Suggest only one (in bst.h) a a reference in bst.c to bst.h to show you did not forget.

  5. Change compare function type to a const version. qsort()'s type shown for reference.

     int (*compare)(const void *, const void *); 
     // qsort's --> int (*compar)(const void *, const void *);
    
  6. Maybe test for compare() against NULL.

  7. To expand compare() possibilities, add a compare state variable.

  8. Concerning callback, allow that function to return an int and use a non-zero value to short-circuit the progress OR provide a state variable for callback().

More later

This is simple a first pass with mostly top level thoughts - I dig deeper later.

  1. Suggest moving all functions to bst.c including bst_init() size_t bst_encode() bst_decode(). By doing so, ENCODING_SIZE_T MAX_CONTENT_LENGTH BST_Node and the fields of BST may be removed from the application view. The ap does not need to see them.

     // BST.h
     typedef struct BST BST;`
    
     // BST.c
     struct BST { 
       BST_Node *root; 
       size_t node_count; 
       int (*compare)(void *, void *);
     };
    
  2. I often use the prefix before functions like BST_This() and BST_That(), but I would be consistent with the filename's case. Use BST.c with BST_This() or bst.c with bst_This().

  3. Need #include <stdlib.h> for size_t in bst.h. You might have caught that if in bst.c you included the standard headers after "bst.h". Suggest moving those after "bst.h". This prevents requiring users of bst.h to include certain headers files first.

  4. Large comment blocks before bst_encode in both bst.h bst.c is a maintenance liability. Suggest only one (in bst.h) a a reference in bst.c to bst.h to show you did not forget.

  5. Change compare function type to a const version. qsort()'s type shown for reference.

     int (*compare)(const void *, const void *); 
     // qsort's --> int (*compar)(const void *, const void *);
    
  6. Maybe test for compare() against NULL.

  7. To expand compare() possibilities, add a compare state variable.

  8. Concerning callback, allow that function to return an int and use a non-zero value to short-circuit the progress OR provide a state variable for callback().

More later

This is simple a first pass with mostly top level thoughts - I dig deeper later.

  1. Suggest moving all functions to bst.c including bst_init() size_t bst_encode() bst_decode(). By doing so, ENCODING_SIZE_T MAX_CONTENT_LENGTH BST_Node and the fields of BST may be removed from the application view. The ap does not need to see them.
    [Edit] I now see bst_init() size_t bst_encode() bst_decode() are only prototyped in bst.h - ignore that part of this suggestion. At first blush, the style looked like code, but it is only prototype.

     // BST.h
     typedef struct BST BST;`
    
     // BST.c
     struct BST { 
       BST_Node *root; 
       size_t node_count; 
       int (*compare)(void *, void *);
     };
    
  2. I often use the prefix before functions like BST_This() and BST_That(), but I would be consistent with the filename's case. Use BST.c with BST_This() or bst.c with bst_This().

  3. Need #include <stdlib.h> for size_t in bst.h. You might have caught that if in bst.c you included the standard headers after "bst.h". Suggest moving those after "bst.h". This prevents requiring users of bst.h to include certain headers files first.

  4. Large comment blocks before bst_encode in both bst.h bst.c is a maintenance liability. Suggest only one (in bst.h) a a reference in bst.c to bst.h to show you did not forget.

  5. Change compare function type to a const version. qsort()'s type shown for reference.

     int (*compare)(const void *, const void *); 
     // qsort's --> int (*compar)(const void *, const void *);
    
  6. Maybe test for compare() against NULL.

  7. To expand compare() possibilities, add a compare state variable.

  8. Concerning callback, allow that function to return an int and use a non-zero value to short-circuit the progress OR provide a state variable for callback().

More later

added 122 characters in body
Source Link
chux
  • 36.5k
  • 2
  • 43
  • 97

This is simple a first pass with mostly top level thoughts - I dig deeper later.

  1. Suggest moving all functions to bst.c including bst_init() size_t bst_encode() bst_decode(). By doing so, ENCODING_SIZE_T MAX_CONTENT_LENGTH BST_Node and the fields of BST may be removed from the application view. The ap does not need to see them.

     // BST.h
     typedef struct BST BST;`
    
     // BST.c
     struct BST { 
       BST_Node *root; 
       size_t node_count; 
       int (*compare)(void *, void *);
     };
    
  2. I often use the prefix before functions like BST_This() and BST_That(), but I would be consistent with the filename's case. Use BST.c with BST_This() or bst.c with bst_This().

  3. Need #include <stdlib.h> for size_t in bst.h. You might have caught that if in bst.c you included the standard headers after "bst.h". Suggest moving those after "bst.h". This prevents requiring users of bst.h to include certain headers files first.

  4. Large comment blocks before bst_encode in both bst.h bst.c is a maintenance liability. Suggest only one (in bst.h) a a reference in bst.c to bst.h to show you did not forget.

  5. Change compare function type to a const version. qsort()'s type shown for reference.

     int (*compare)(const void *, const void *); 
     // qsort's --> int (*compar)(const void *, const void *);
    
  6. Maybe test for compare() against NULL.

  7. To expand compare() possibilities, add a compare state variable.

  8. Concerning callback, allow that function to return an int and use a non-zero value to short-circuit the progress OR provide a state variable for callback().

More later

This is simple a first pass with mostly top level thoughts - I dig deeper later.

  1. Suggest moving all functions to bst.c including bst_init() size_t bst_encode() bst_decode(). By doing so, ENCODING_SIZE_T MAX_CONTENT_LENGTH BST_Node and the fields of BST may be removed from the application view. The ap does not need to see them.

     // BST.h
     typedef struct BST BST;`
    
     // BST.c
     struct BST { 
       BST_Node *root; 
       size_t node_count; 
       int (*compare)(void *, void *);
     };
    
  2. I often use the prefix before functions like BST_This() and BST_That(), but I would be consistent with the filename's case. Use BST.c with BST_This() or bst.c with bst_This().

  3. Need #include <stdlib.h> for size_t in bst.h. You might have caught that if in bst.c you included the standard headers after "bst.h". Suggest moving those after "bst.h". This prevents requiring users of bst.h to include certain headers files first.

  4. Large comment blocks before bst_encode in both bst.h bst.c is a maintenance liability. Suggest only one (in bst.h) a a reference in bst.c to bst.h to show you did not forget.

  5. Change compare function type to a const version. qsort()'s type shown for reference.

     int (*compare)(const void *, const void *); 
     // qsort's --> int (*compar)(const void *, const void *);
    

More later

This is simple a first pass with mostly top level thoughts - I dig deeper later.

  1. Suggest moving all functions to bst.c including bst_init() size_t bst_encode() bst_decode(). By doing so, ENCODING_SIZE_T MAX_CONTENT_LENGTH BST_Node and the fields of BST may be removed from the application view. The ap does not need to see them.

     // BST.h
     typedef struct BST BST;`
    
     // BST.c
     struct BST { 
       BST_Node *root; 
       size_t node_count; 
       int (*compare)(void *, void *);
     };
    
  2. I often use the prefix before functions like BST_This() and BST_That(), but I would be consistent with the filename's case. Use BST.c with BST_This() or bst.c with bst_This().

  3. Need #include <stdlib.h> for size_t in bst.h. You might have caught that if in bst.c you included the standard headers after "bst.h". Suggest moving those after "bst.h". This prevents requiring users of bst.h to include certain headers files first.

  4. Large comment blocks before bst_encode in both bst.h bst.c is a maintenance liability. Suggest only one (in bst.h) a a reference in bst.c to bst.h to show you did not forget.

  5. Change compare function type to a const version. qsort()'s type shown for reference.

     int (*compare)(const void *, const void *); 
     // qsort's --> int (*compar)(const void *, const void *);
    
  6. Maybe test for compare() against NULL.

  7. To expand compare() possibilities, add a compare state variable.

  8. Concerning callback, allow that function to return an int and use a non-zero value to short-circuit the progress OR provide a state variable for callback().

More later

Source Link
chux
  • 36.5k
  • 2
  • 43
  • 97

This is simple a first pass with mostly top level thoughts - I dig deeper later.

  1. Suggest moving all functions to bst.c including bst_init() size_t bst_encode() bst_decode(). By doing so, ENCODING_SIZE_T MAX_CONTENT_LENGTH BST_Node and the fields of BST may be removed from the application view. The ap does not need to see them.

     // BST.h
     typedef struct BST BST;`
    
     // BST.c
     struct BST { 
       BST_Node *root; 
       size_t node_count; 
       int (*compare)(void *, void *);
     };
    
  2. I often use the prefix before functions like BST_This() and BST_That(), but I would be consistent with the filename's case. Use BST.c with BST_This() or bst.c with bst_This().

  3. Need #include <stdlib.h> for size_t in bst.h. You might have caught that if in bst.c you included the standard headers after "bst.h". Suggest moving those after "bst.h". This prevents requiring users of bst.h to include certain headers files first.

  4. Large comment blocks before bst_encode in both bst.h bst.c is a maintenance liability. Suggest only one (in bst.h) a a reference in bst.c to bst.h to show you did not forget.

  5. Change compare function type to a const version. qsort()'s type shown for reference.

     int (*compare)(const void *, const void *); 
     // qsort's --> int (*compar)(const void *, const void *);
    

More later