CODEWITHSUNDEEP
c++linked-listset
user2793442
user2793442

Reputation: 129

Testing if a linked list is equal to each other

I have a set of numbers that are in a linked list. I want to compare them to see if they are the same numbers in a set. Here is my code right now:

bool set::equalset(set second)
{
     Data *travel1, *travel2;
     travel1 = top;
     travel2 = second.topval(); //gets the top value for the second set
     while (travel2->next != NULL && travel1->next != NULL)
     {
           if (!in(travel2->value))  //in checks if the value is in the set
               return false;
           if (!second.in(travel1->value)) //same idea here
               return false;

           travel2 = travel2->next;
           travel1 = travel1->next;

           return true;
     }
     return false;
}

So what my code does is grab the top values for both of the sets and sets those equal to travel1/2 respectively, then while travel doesn't point to a null value in both sets, it traverses the lists and checks if values from either of the sets are in each other. If a value is not found, it sets it to false. Otherwise, it will be set to true and they are found to be equal.

However, this code only half works - you can easily break it by entering 1, 2 for the first set and 1, 2, 3 for the second set and they will be returned as equal. I would think that the third value (3) would make it return false. What is the missing link here?

Upvotes: 1

Views: 2445

Answers (4)

WhozCraig
WhozCraig

Reputation: 66194

Your code has several problems. First, you're not checking the last node. A loop condition like this:

while (travel2->next != NULL && travel1->next != NULL)

will break as soon as one of the enumerators reaches the last node, but never checks it. Further, it will also mean two sets of a single-node-each will always compare true.

Next, You have a hard return after only the first iteration, so there is no conceivable way this ever returned false on two sets that started with the same node value.

travel2 = travel2->next;
travel1 = travel1->next;

return true; // this doesn't belong here.

Next, you pass your parameter by-value, which means the copy constructor is being invoked. I don't know whether you implemented it or not (and if you didn't, you have an entire different problem), but there is no reason to duplicate the list just to see if it is equal to *this*. The function should take a const-reference as the parameter.

bool set::equalset(const set& second)

Finally, your exit condition is correct, but you cannot assume the lists were both exhausted. you have to verify it. You can do this by returning false if either traveler is non-null (and one of them will be if the lists are uneven.

Putting it all together:

bool set::equalset(const set& second)
{
     const Data *travel1 = top;
     const Data *travel2 = second.top;
     while (travel1 && travel2)
     {
           if (!in(travel2->value))  //in checks if the value is in the set
               return false;
           if (!second.in(travel1->value)) //same idea here
               return false;

           travel2 = travel2->next;
           travel1 = travel1->next;
     }
     return !(travel1 || travel2);
}

Optimized for Sorted Lists

If you keep the lists sorted during input and removal methods, you can significantly make this easier, seen below:

bool set::equalset(const set& second)
{
     const Data *travel1 = top;
     const Data *travel2 = second.top;
     while (travel1 && travel2 && travel1->value == travel2->value)
     {
           travel1 = travel1->next;
           travel2 = travel2->next;
     }
     return !(travel1 || travel2);
}

Upvotes: 1

999k
999k

Reputation: 6565

with your code if two sets have different number of members. your loop will exit as travel1 or travel2 which has less no. of elements will point to NULL, when other is still not NULL. In your case travel1 will point to NULL and there will still elements to parse in travel2

Check by below code

bool set::equalset(set second)
{
     Data *travel1, *travel2;
     travel1 = top;
     travel2 = second.topval(); //gets the top value for the second set
     while (travel2->next != NULL && travel1->next != NULL)
     {
           if (!in(travel2->value))  //in checks if the value is in the set
               return false;
           if (!second.in(travel1->value)) //same idea here
               return false;

           travel2 = travel2->next;
           travel1 = travel1->next;


     }
     if (travel2->next == NULL && travel1->next == NULL)
     {
      return true;
     }
     return false;
}

In this code for two set with elements 1 2 2 and 1 2 it will return false

Upvotes: 1

RouteMapper
RouteMapper

Reputation: 2560

You need to adjust your return conditions because as of now, you return true if the first value in both this and second are present in each list.

Do something like this instead:

bool set::equalset(set second)
{
     Data *travel1, *travel2;
     travel1 = top;
     travel2 = second.topval(); //gets the top value for the second set

     while (travel2->next != NULL && travel1->next != NULL)
     {
           if (!in(travel2->value))  //in checks if the value is in the set
               return false;
           if (!second.in(travel1->value)) //same idea here
               return false;

           travel2 = travel2->next;
           travel1 = travel1->next;
     }
     return true;
}

Upvotes: 1

user2986089
user2986089

Reputation: 46

In the situation you describe, the first set's iterator will be NULL before the third element of the second set is searched for, breaking your while loop. You may as well loop through each set independently. You could also check if both sets have the same number of elements before comparing the elements of each.

Upvotes: 1

Related Questions