Skip to main content
Spelling and grammar
Source Link
Toby Speight
  • 88.7k
  • 14
  • 104
  • 327

Answers to Questions:

  1. Optimizations - Is my code sufficiently optimized and if any more optimization is possible.

Optimization is the last thing to worry about once the code is written properly and working as expected. Most of the optimization can acomplishedaccomplished be by using the -O3-O3 compiler switch,switch; it generally does a better job of optimization than optimizing code by hand.

  1. Memory leaks - Is there any scope for memory leaks.

Whether or not there are memory leaks is academic at this time, there is a buffer overflow problem in the function static void inorder_helper(TreeNode* root, char** inorder_string). A string allocated by strdup() will not cotaincontain enough space for additional strings to be concatenated. The entire inorderin-order traversal needs to be redesigned and or reimplemented.

  1. Readability

The readability is fair, the variable and function names are good. The readability could be improved by adding some vertical spacing between the functions and between logic blocks within the functions.

Proper Header Files for a Library

Only include in the header file what is necessary for files accessing the library to compile properly. Right now the header file contains things that aren't necessary and in some cases may cause problems for linking or compiling. The file tree.c should contain the header files that are currently included by tree.h. The only code tree.htree.h should contain are the definitions of the tree and the 3 global entry point functions.


#ifndef TREE_H
#define TREE_H

typedef struct tree_node {
    int val;
    struct tree_node* left, * right;
} TreeNode;

typedef struct tree {
    TreeNode* root;
} Tree;

char* inorder(Tree*);
Tree* makeTree(char*);
void deleteTree(Tree*);

#endif

The static declarations for all the functions aren't necessary for code utilizing the library and could collide with functions in the modules that are including the library.

The lowercase null definition is only necessary in tree.c and should be declared there, although I would really recommend that you just stick with the original definition of NULL.

The current include guard #ifndef TREE_H is not effective and will allow most of the file to be included multiple times.

Answers to Questions:

  1. Optimizations - Is my code sufficiently optimized and if any more optimization is possible.

Optimization is the last thing to worry about once the code is written properly and working as expected. Most of the optimization can acomplished be by using the -O3 compiler switch, it generally does a better job of optimization than optimizing code by hand.

  1. Memory leaks - Is there any scope for memory leaks.

Whether or not there are memory leaks is academic at this time, there is a buffer overflow problem in the function static void inorder_helper(TreeNode* root, char** inorder_string). A string allocated by strdup() will not cotain enough space for additional strings to be concatenated. The entire inorder traversal needs to be redesigned and or reimplemented.

  1. Readability

The readability is fair, the variable and function names are good. The readability could be improved by adding some vertical spacing between the functions and between logic blocks within the functions.

Proper Header Files for a Library

Only include in the header file what is necessary for files accessing the library to compile properly. Right now the header file contains things that aren't necessary and in some cases may cause problems for linking or compiling. The file tree.c should contain the header files that are currently included by tree.h. The only code tree.h should contain are the definitions of the tree and the 3 global entry point functions.


#ifndef TREE_H
#define TREE_H

typedef struct tree_node {
    int val;
    struct tree_node* left, * right;
} TreeNode;

typedef struct tree {
    TreeNode* root;
} Tree;

char* inorder(Tree*);
Tree* makeTree(char*);
void deleteTree(Tree*);

#endif

The static declarations for all the functions aren't necessary for code utilizing the library and could collide with functions in the modules that are including the library.

The lowercase null definition is only necessary in tree.c and should be declared there, although I would really recommend that you just stick with the original definition of NULL.

The current include guard #ifndef TREE_H is not effective and will allow most of the file to be included multiple times.

Answers to Questions:

  1. Optimizations - Is my code sufficiently optimized and if any more optimization is possible.

Optimization is the last thing to worry about once the code is written properly and working as expected. Most of the optimization can accomplished be by using the -O3 compiler switch; it generally does a better job of optimization than optimizing code by hand.

  1. Memory leaks - Is there any scope for memory leaks.

Whether or not there are memory leaks is academic at this time, there is a buffer overflow problem in the function static void inorder_helper(TreeNode* root, char** inorder_string). A string allocated by strdup() will not contain enough space for additional strings to be concatenated. The entire in-order traversal needs to be redesigned and or reimplemented.

  1. Readability

The readability is fair, the variable and function names are good. The readability could be improved by adding some vertical spacing between the functions and between logic blocks within the functions.

Proper Header Files for a Library

Only include in the header file what is necessary for files accessing the library to compile properly. Right now the header file contains things that aren't necessary and in some cases may cause problems for linking or compiling. The file tree.c should contain the header files that are currently included by tree.h. The only code tree.h should contain are the definitions of the tree and the 3 global entry point functions.


#ifndef TREE_H
#define TREE_H

typedef struct tree_node {
    int val;
    struct tree_node* left, * right;
} TreeNode;

typedef struct tree {
    TreeNode* root;
} Tree;

char* inorder(Tree*);
Tree* makeTree(char*);
void deleteTree(Tree*);

#endif

The static declarations for all the functions aren't necessary for code utilizing the library and could collide with functions in the modules that are including the library.

The lowercase null definition is only necessary in tree.c and should be declared there, although I would really recommend that you just stick with the original definition of NULL.

The current include guard #ifndef TREE_H is not effective and will allow most of the file to be included multiple times.

Source Link
pacmaninbw
  • 26.2k
  • 13
  • 47
  • 114

Answers to Questions:

  1. Optimizations - Is my code sufficiently optimized and if any more optimization is possible.

Optimization is the last thing to worry about once the code is written properly and working as expected. Most of the optimization can acomplished be by using the -O3 compiler switch, it generally does a better job of optimization than optimizing code by hand.

  1. Memory leaks - Is there any scope for memory leaks.

Whether or not there are memory leaks is academic at this time, there is a buffer overflow problem in the function static void inorder_helper(TreeNode* root, char** inorder_string). A string allocated by strdup() will not cotain enough space for additional strings to be concatenated. The entire inorder traversal needs to be redesigned and or reimplemented.

  1. Readability

The readability is fair, the variable and function names are good. The readability could be improved by adding some vertical spacing between the functions and between logic blocks within the functions.

Proper Header Files for a Library

Only include in the header file what is necessary for files accessing the library to compile properly. Right now the header file contains things that aren't necessary and in some cases may cause problems for linking or compiling. The file tree.c should contain the header files that are currently included by tree.h. The only code tree.h should contain are the definitions of the tree and the 3 global entry point functions.


#ifndef TREE_H
#define TREE_H

typedef struct tree_node {
    int val;
    struct tree_node* left, * right;
} TreeNode;

typedef struct tree {
    TreeNode* root;
} Tree;

char* inorder(Tree*);
Tree* makeTree(char*);
void deleteTree(Tree*);

#endif

The static declarations for all the functions aren't necessary for code utilizing the library and could collide with functions in the modules that are including the library.

The lowercase null definition is only necessary in tree.c and should be declared there, although I would really recommend that you just stick with the original definition of NULL.

The current include guard #ifndef TREE_H is not effective and will allow most of the file to be included multiple times.