Linked list subsets

[birb]

Hello everyone, I wrote a function to check if one linked list is a subset of the other one, but I'm new to oop and using list, so I'm pretty unsure about my code. I would really appreciate, if you could check that there is nothing extremely stupid that I missed or maybe if there is some way to simplify it.
Here it is:

  bool Equal(List &L1, List &L2) {
	

	if (L1.GetHead() == NULL || L2.GetHead() == NULL) return true;
	
	int length1 = L1.Count();
	int length2 = L2.Count();

	L1.Sort();
	L2.Sort();

	List::ListItem* list1 = L1.GetHead(); //is this a good way of getting the list items from the class?
	List::ListItem* list2 = L2.GetHead();

	int counter = 0;
	
	if (length1 > length2) {

	
		while (list1 != NULL && list2 != NULL) {

			if (list1->Value == list2->Value) {
				list1 = list1->Next;
				list2 = list2->Next;
				counter++;
			}
			else if (list1->Value < list2->Value) {
				list1 = list1->Next;
			}
			else return false;

		}

		if (counter < length2) return false;

	}
	else {

		while (list1 != NULL && list2 != NULL) {

			if (list1->Value == list2->Value) {
				list1 = list1->Next;
				list2 = list2->Next;
				counter++;
			}
			else if (list2->Value < list1->Value) {
				list2 = list2->Next;
				
			}
			else return false;

		}

		if (counter < length1) return false;
	}
	return true;
}

[jonnin]

it depends on how you define subset, I guess.
list 1,2,3 and list 1,3 … the second is a subset in terms of set notation, but there is no place you can 'cut' list 1 to produce list 2.

my first question is what do you get with this:
1,1,1,2,3 and 1,3,3 ? if you get false, why? Is false what YOU want here?


my second question is why you repeated a block of code. You can avoid this 2 ways... 1 would be to swap a pointer if 2 is longer than 1 and then after that check/assign the same block will work. 2nd method is a single recursive call where you reverse the input args and return the result of that if 2 is longer than 1. Its not wrong, but repeating the same code too many times is cluttered and hard to catch small changes. Twice... its not a big deal. This would simplify it (though not going to run any faster or anything). Since you already have pointers, swapping them is the way to go here.

[birb]

@jonnin thanks for the answer, I'm looking at the first case as a subset, but if I wouldn't I can just find the first element of the shorter list in the longer one and then all the following elements have to match until I'm at the end, right?

The second will never happen to me, because the lists do not contain duplicites, I forgot to mention that:)

I've even tried to swap the lists, but I couldn't figure it out, bcs i used this:

if(length2>length1){
List tmp=L1;
L1=L2;
L2=tmp;
}
On my way to fix it.

[jonnin]

no swap the pointers.
List::ListItem* list1 = L1.GetHead();
List::ListItem* list2 = L2.GetHead();

//you will love it :P
if (length1 > length2)
{
list2 = L1.GetHead(); //cheating swap, no temp required !
list1 = L2.GetHead();
}

which of course means you could also rewire the list1/list2 initialization to be based off the lengths in the first place.. but its a bit of syntax abuse for no good reason to do that.

[birb]

I also had to swap the lengths, but suddenly it looks much better, thanks a lot!