Howdy, Stranger!

It looks like you're new here. If you want to get involved, click one of these buttons!

Categories

Welcome to the new platform of Programmer's Heaven! We apologize for the inconvenience caused, if you visited us from a broken link of the previous version. The main reason to move to a new platform is to provide more effective and collaborative experience to you all. Please feel free to experience the new platform and use its exciting features. Contact us for any issue that you need to get clarified. We are more than happy to help you.

Cant seem to see what is not good in the code.

arklevarklev israelPosts: 0Member
edited May 22 in Beginner C/C++

Ok, here is the question.

You have two lists that are already sorted, you have to merge them and return a new list without any new extra nodes. The returned list should be sorted as well.

 #include <stdio.h>   
 #include <stdlib.h>   

 #pragma warning(disable: 4996)

 typedef struct item 
 { 
    int data; 
    struct item *next; 
 }Item;

 Item *addToLastValue(int value,Item* head);
 Item *sort_list(Item *lst1,Item *lst2);
 void printList(Item *head);

void main(){
    Item *head=NULL;
    Item *head2=NULL;
    Item *sorted=NULL;
    head=addToLastValue(5,head); 
    head=addToLastValue(3,head); 
    head=addToLastValue(1,head);
    head2=addToLastValue(6,head2); 
    head2=addToLastValue(4,head2); 
    head2=addToLastValue(2,head2);
    printList(head);
    printf("\n");
    printList(head2);
    printf("\n");
    sorted = sort_list(head,head2);
    printList(sorted);
    printf("\n");
 }
 Item *addToLastValue(int value,Item* head) 
 { 
    Item *temp; 
    if (head==NULL) 
    { 
        head=(Item*) malloc(sizeof(Item)); 
        head->data=value; 
        head->next=NULL; 
        return head; 
    } 
    temp=(Item*) malloc(sizeof(Item)); 
    temp->data=value; 
    temp->next=head; 
    return temp; 
 }
 Item *sort_list(Item *list1,Item *list2){
    Item *temp=NULL;
    Item *head=NULL;
    if(list1 == NULL && list2 ==NULL)
        return NULL;
    if(list1=NULL)
        return list2;
    if(list2=NULL)
        return list1;
    if (list1->data < list2->data)
    {
        temp = list1;
        list1 = list1->next;
    }
    else
    { 
        temp = list2;
        list2 = list2->next;
    }
    head = temp;
    while(1){
        if(list1->data < list2->data){
            temp->next = list1;
            temp = temp->next;
            list1 = list1->next;
        }
        else if(list1->data > list2->data){
            temp->next = list2;
            temp = temp->next;
            list2 = list2->next;
        }
        if (list1 == NULL){
            temp->next = list1;
            break;
        }
        else if (list2 == NULL){
            temp->next = list2;
            break;
        }
    }
    return head;
 }
 void printList(Item *head){  
    Item *temp;  
    temp = head;  
    while(temp != NULL){  
        printf("[%d]->", temp->data);  
    temp = temp->next;  
    }  
 }

Comments

  • HK_MP5KPDWHK_MP5KPDW Posts: 770Member ✭✭✭

    There is no question in your post, only a statement followed by some code. Have you compiled your code? Gotten any warning/error messages? If so what where they? If you got it to compile/link then what happened when you ran it? What results did you get versus what you expected to find?

    At a glance:

    1. A lot of purists will get upset at the return value of your main function. main should always return an int, not void.
    2. A bit of a nit but addToLastValue is a confusing name for the function given what is does. Perhaps a more appropriate name would be addToFront since it effectively adds the value to the front of the existing list.
    3. There should be no reason (at least in C) to cast the return value of the malloc function. The void pointer that it returns can be safely converted into other pointer types implicitly without need of an explicit cast. It is preferable to not cast this return value.
    4. You do not check if your malloc calls succeed prior to using the pointers, you simply assume they worked without issue and go on operating on that assumption throughout your code.
    5. There are several places where you seem to have forgotten the difference between the assignment operator (=) and the boolean true/false equality operator (==).
    6. You don't seem to account for the case where the values in the two lists are equal, only where one or the other list's value is greater.
    7. Your (list1/list2==null) check at the end of the loop does not correctly terminate the combined list. Draw a diagram on paper of your two lists and work through your algorithm step by step.
  • tienkhoanguyentienkhoanguyen houPosts: 64Member

    Jesus Christ!hehe Well, I prefer to be positive so I will just say "Good job." You have codings that I do not even know. You know something I do not. Congratulations. I do not know that much to begin with since I have forgotten much of my programming days. However, to each there own. If it works for you and gets the programming you desire done, I hope you continue using it even if it seems odd to me or the rest of the world. I am too old to learn new techniques. I just program what comes out of my mind. It usually works after I have error checked it. However, I am happy I succeeded in doing something that would hopefully make both my parents happy. Maybe I can even bring in a living doing it one day not only for myself but also for all my love ones. Just my two cents that I owe all haha Enjoy your life either way buddy boy (no offense intended since I say this to my closes love one who is my very own cousin who I consider as a brother). Enjoy and God bless us all

Sign In or Register to comment.