-2

The code below,

part is one of ('frodo', 'fradi'), ('frodo', 'crodo'), ('frodo', 'abc123'), ('frodo', 'abc123'), ('frodo', 'frodoc'), ('fradi', 'crodo'), ('fradi', 'abc123'), ('fradi', 'abc123'), ('fradi', 'frodoc'), ('crodo', 'abc123').

ban_group is ["fr*d*", "abc1**"]

Function solution checks if part[i] can be ban_group[i].

What I expected was

when part is ('frodo', 'fradi'), part[0] can be ban_group[0] and part[1] can't be ban_group[1]. So it's False.

when part is ('frodo', 'abc123'), part[0] can be ban_group[0] and part[1] can be ban_group[1]. So it's True, so add part to list candidates.

But it doesn't work as I expected. I really don't get what part is problem.

Can you help me with this?

from itertools import combinations

def check_id(user, ban):
    for i in range(len(ban)):
        if ban[i] == '*':
            continue
        elif user[i] != ban[i]:
            return False
    return True

candidates = []
def solution(user_group, ban_group):
    for part in combinations(user_group, len(ban_group)):
        for i in range(len(part)):
            if check_id(part[i], ban_group[i]) is True:
                candidates.append(part)
    return candidates

print(solution(["frodo", "fradi", "crodo", "abc123", "frodoc"], ["fr*d*", "abc1**"]))
Yellowgreen
  • 1
  • 1

3 Answers3

0

In this fragment:

        for i in range(len(part)):
            if check_id(part[i], ban_group[i]) is True:
                candidates.append(part)

you append part to candidates if any item in part passed check_id. I think you wanted to do it only if all items in part passed check_id. So, the correct thing to do is:

        if all(check_id(part[i], ban_group[i]) for i in range(len(part))):
            candidates.append(part)

BTW, Karl Knechtel is right about check_id function - it is not correct either, just see what check_id('frodoc', 'fr*d*') does.

Błotosmętek
  • 12,717
  • 19
  • 29
  • Can I ask you why it can be 'any'? if i iterate 0 to 2, I thought this function pass in only (part[0], ban_group[0]), (part[1], ban_group[1]). cause i works identically in part and ban_group at the same time. If it's kind of any, does it mean it pass (part[0], ban_group[1]) too? – Yellowgreen May 12 '20 at 18:11
0
    for i in range(len(part)):
        if check_id(part[i], ban_group[i]) is True:
            candidates.append(part)

This will compare each element from part to the corresponding element of ban_group, and append the entire part to candidates for each element that matched.

What you want (if I understood you correctly) is to check each element from part against the entire ban_group, to see if any of the ban_group elements matched. Then you need to look at those results and see if all of the part elements matched, and possibly append once to the candidates.

It can be tricky to do this work with for loops, and takes a lot of code for a simple idea. Much better is to use the built-in any and all functions, which can be passed a generator expression in order to process an entire list easily. Then you can use functions to name each step of the process. For the overall loop, we can also use a list comprehension instead of appending each result to a new list.

def is_banned(user, ban_group):
    # use `any` to see if the user matches any of the ban patterns.
    return any(check_id(user, ban) for ban in ban_group)

def all_banned(part, ban_group):
    # use `all` to see if all of the users matched a ban pattern.
    return all(is_banned(user, ban_group) for user in part)

def solution(user_group, ban_group):
    # apply the `all_banned` logic to each combination of users of the
    # specified size.
    return [
        part for part in combinations(user_group, len(ban_group))
        if all_banned(part, ban_group)
    ]

I hope I correctly understand the intended logic. If not, it should be easy enough to fix, using similar technique.

Karl Knechtel
  • 62,466
  • 11
  • 102
  • 153
  • It occurred to me, maybe you *did* want to compare a given user to a corresponding ban pattern. For this purpose, the built-in `zip` is helpful - see for example https://stackoverflow.com/questions/1663807/how-to-iterate-through-two-lists-in-parallel . However, be careful if the order matters - notice the difference between `itertools.combinations` and `itertools.permutations`. – Karl Knechtel May 11 '20 at 22:13
  • Thanks so much. It's really helpful!! – Yellowgreen May 11 '20 at 22:27
0

If I understand currectly, You want to append a part to candidate = [] only if all element of matches with their co-respondend position element in ban group. But in your code you are appending part to candidate if any one of those element matched not both of them. That's why I have use a flag to keep track if all of the element in part matched.

from itertools import combinations
def check_id(user, ban):
    for i in range(len(ban)):
        if ban[i] == '*':
            continue
        elif user[i] != ban[i]:
            return False
    return True

candidates = []
def solution(user_group, ban_group):
    for part in combinations(user_group, len(ban_group)):
        flag = True
        for i in range(len(part)):
            flag = flag and check_id(part[i], ban_group[i])
        if flag:
            candidates.append(part)
    return candidates

print(solution(["frodo", "fradi", "crodo", "abc123", "frodoc"], ["fr*d*", "abc1**"]))
Plaxus
  • 1
  • 1
  • 2