cbinary-treepass-by-referencepass-by-valuedatamember

makeNode function. what's wrong with it?


I created 2 struct dl and Node. I used struct dl for created Node and created function makeNode. Please help me! Thanks you guys read it.

typedef struct _dl {
    char ProblemID[10];
    char TimePoint[20];
    char Status[10];
    int Point;
} dl;

typedef struct _Node {
    dl x;
    struct _Node* leftChild;
    struct _Node* rightChild;
} Node;

void cpy(dl x, dl a) {
    strcpy(x.ProblemID, a.ProblemID);
    strcpy(x.TimePoint, a.TimePoint);
    strcpy(x.Status, a.Status);
    x.Point = a.Point;
    printf("%s %s %s %d\n",x.ProblemID,x.TimePoint,x.Status,x.Point);
}

Node* makeNode(dl a) {
    Node* p = (Node*)malloc(sizeof(Node));
    if (p == NULL) {
        exit(1);
    }
    cpy(p->x, a);
    printf("%s %s %s %d\n",p->x.ProblemID,p->x.TimePoint,p->x.Status,p->x.Point);
    p->leftChild = NULL;
    p->rightChild = NULL;
    return p;
}

Can you point out my mistakes? Thanks for all.


Solution

  • The function cpy deals with copies of values of passed argument expressions. So it actually does not change the data member x of the created node.

    Instead you should write at least

    void cpy( dl *x, const dl *a) {
        strcpy(x->ProblemID, a->ProblemID);
        strcpy(x->TimePoint, a->TimePoint);
        strcpy(x->Status, a->Status);
        x->Point = a->Point;
        printf("%s %s %s %d\n",x->ProblemID,x->TimePoint,x->Status,x->Point);
    }
    

    and the function cpy is called within makeNode that should be declared like

    Node* makeNode( const dl *a);
    

    the following way

    cpy( &p->x, a);
    

    Pay attention to that it would be better if to remove the statement with exit Instead the function can return a null pointer and the caller of the function could check the returned value. For example

    Node* makeNode( const dl *a ) 
    {
        Node *p = malloc( sizeof( *p ) );
    
        if ( p != NULL )
        {
            cpy( &p->x, a );
            //printf("%s %s %s %d\n",p->x.ProblemID,p->x.TimePoint,p->x.Status,p->x.Point);
            p->leftChild  = NULL;
            p->rightChild = NULL;
        }
    
        return p;
    }