HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Snippet of an easy binary search tree

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
snippetsearcheasybinarytree

Problem

ALL I want to do is making binary search tree, inputting there from users, and then print out data in each node one by one according to the user's selection (whether he choose to print the data in the right node of the current node or the left). So I only define three functions: prompt(), inputData() and printOneNode().

As you can see, I place the prompt() function in an infinite while loop and continuously prompt the user for selection, until he chooses to exit.

I have also made some comments in the source code for improvement of my code.

```
#include
#include
#include

typedef struct BSTnode{
struct BSTnode* leftchild;
struct BSTnode* rightchild;
char data[20];
}BSTnode;

void prompt();
void inputData();
void printOneNode();

BSTnode firstNode;
BSTnode* MyNode=&firstNode; //I don't know if there is another better way to initialize the first node

int main()
{
//BSTnode* MyNode=malloc(sizeof(BSTnode)); //Can't initialize it in the main function , or it won't be accessible in another function

MyNode->leftchild=NULL;
MyNode->rightchild=NULL;
strcpy(MyNode->data,"");
while(1)
prompt();

return 0;
}

void prompt(){
int i=0;
printf("Please select:\n");
printf("1.input a data\n2.print only one data\n3.Exit\n");

scanf("%d",&i);
switch(i){
case 1:
inputData();
break;
case 2:
printOneNode();
break;
case 3:
exit(0);
default:
printf("Please input a valid number!(1-3)");
}
}

void inputData(){
char data[20]={0};
printf("Input your data here(character only / less than 19 characters!): ");
scanf("%s",data);
BSTnode** ptr=&MyNode;
while(1){
if(strcmp(data,(*ptr)->data)){
if((*ptr)->rightchild!=NULL){
ptr=&((*ptr)->rightchild);
continue;
}
else{
(*ptr)->rightchild=mallo

Solution

//BSTnode* MyNode=malloc(sizeof(BSTnode));    //Can't initialize it in the main function , or it won't be accessible in another function


I want to quickly list things wrong with this line. Most of these are very minor, but it's a lot for one line. Further, the big problem is particularly big.

Minor

-
Trailing space.

-
The = isn't spaced (nor is the //).

-
Typo in the comment (space before the comma).

-
Inconsistent naming. You write lowerCamelCase elsewhere for local variables, but use UpperCamelCase for this one. You also write BSTnode but MyNode. This should probably by BSTNode and myNode.

Significant

-
It's way too long.

-
There's no NULL check for malloc.

Big

  • To make things accessible in other functions, pass it as a parameter. Do not use globals for this purpose.



Ideally one would do something like

int main() {
    // calloc automatically zeroes the fields
    BSTNode* myNode = calloc(1, sizeof(BSTNode));
    if (!myNode) {
        // Handle allocation failure by bailing.
        // Anything sensible is fine.
        return 1;
    }
    while(1) {
        prompt(myNode);
    }
}

Code Snippets

//BSTnode* MyNode=malloc(sizeof(BSTnode));    //Can't initialize it in the main function , or it won't be accessible in another function
int main() {
    // calloc automatically zeroes the fields
    BSTNode* myNode = calloc(1, sizeof(BSTNode));
    if (!myNode) {
        // Handle allocation failure by bailing.
        // Anything sensible is fine.
        return 1;
    }
    while(1) {
        prompt(myNode);
    }
}

Context

StackExchange Code Review Q#94152, answer score: 5

Revisions (0)

No revisions yet.