Is there a way I can make this more elegant? I cant figure out any other way to write this-CodePudding

comboList = [[0,1,2],[3,4,5],[6,7,8],[0,3,6],[1,4,7],[2,5,8],[0,4,8],[2,4,6]]
#duplicate values wont be entered into these test lists to begin with so idc about that
testList1 = [0,1,2]
testList2 = [1,2,4,7] 
testList3 = [0,2,3,6,5,69,4,6,1]
testList4 = [2,1,3] #this needs to return false

def testfunc(mainList, sublist):#This is the trash func
    for list in mainList: 
        y1 = 0
        x1 = 0
        while x1 < len(sublist):
            if sublist[x1] in list:
                y1 = y1   1
                if y1 == 3:
                    return True
            x1 = x1   1
    return False

if testfunc(comboList,testList1):
    print("Test1 Pass")
else:
    print("Test1 Fail")

if testfunc(comboList,testList2):
    print("Test2 Pass")
else:
    print("Test2 Fail")

if testfunc(comboList,testList3):
    print("Test3 Pass")
else:
    print("Test3 Fail")

if testfunc(comboList,testList4):
    print("Test4 Fail")
else:
    print("Test4 Pass")

I am fairly new to to this and I would like some feedback on how to write this more elegantly, this function is currently doing exactly what I want it to do but there should be a better way of doing it especially in python.

CodePudding user response：

You could break down the logic into separate functions which are shorter and more readable, like this:

def has_n_items(iterable, n):
    """ Check whether an iterable has at least n items """
    for i, _ in enumerate(iterable, 1):
        if i == n:
            return True
    return False

def has_three_members(a, b):
    """ Test whether at least three members of b are found in a """
    return has_n_items((item for item in b if item in a), 3)
            
def test_func(a, b):
    """ Test whether at least three members of list b are found in any of the sublists of list of lists a """
    return any(has_three_members(b, items) for sublist in a)

CodePudding user response：

As well as improving your function you could simplify the test to avoid repeats:

comboList = [[0,1,2],[3,4,5],[6,7,8],[0,3,6],[1,4,7],[2,5,8],[0,4,8],[2,4,6]]
#duplicate values wont be entered into these test lists to begin with so idc about that

testlists = {'test1': [0,1,2], 'test2': [1,2,4,7], 'test3': [0,2,3,6,5,69,4,6,1], 'test4': [2,1,3]}

def testfunc(mainList, sublist):
    for mylist in mainList:
        count = 0
        for item in sublist:
            if item in mylist:
                count  = 1
            if count == 3:
                return True
    return False

for name, test in testlists.items():
    if testfunc(comboList, test):
        print(name, ' Passed')
    else:
        print(name, ' Failed')

CodePudding user response：

for improving your function you can do this:


def testfunc(mainList,sublist): 
    for l in mainList: 
        if sum(x in l for x in sublist) == len(l):
            return True
    return False

it pass the tests but idk if that's exactly what you want to do:

check for every val in sublist if match with values in one list of mainList. But checking your code and mine (do the same). it will return True if for example you use as sublist= [0,0,0]

CodePudding user response：

Any time you are testing membership amongst groups, you should be thinking of using sets. If you have control over the original inputs, you should construct them as sets. If you "receive" lists, you can convert them on the fly. This will be galactically faster for "large" data. For tiny examples such as yours, the speedup will not be noticed.

def member_test(main, test_list):
    return any(len(set(sub) & set(test_list)) >= 3 for sub in main)

This uses the any construct and set intersection to do the heavy lifting and forgoes the looping with a compact representation.