Skip to content

Conversation

@Stektpotet
Copy link
Collaborator

Didn't take the time to sit down and get these solutions added on the day, but here they are!

@Stektpotet Stektpotet marked this pull request as ready for review December 10, 2020 18:07
Copy link
Owner

@Arxcis Arxcis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

08,09,10 delivered with glans! 🤩

Comment on lines +21 to +22
if s > magic_number:
break
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha najs optimization 👏

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if not any(s == lines[p+25] for s in map(sum, itertools.combinations(lines[p: p+25], 2))):

This one is pretty cool too imo:

because any accepts iterators, hence also generators or generator expressions, such as above, it will not exhaust the for loop when s==lines[p+25] is met for any single instance of s. mapalso behaves the same way, meaning that I'm effectively...

  1. not summing any more combinations than neccessary, and
  2. not creating any more combinations from the preamable.

making that line more effective than my original loop:

for p in range(len(lines) - 26):
    found=False
    for a, b in itertools.combinations(lines[p: p+25], 2):
        if a + b == lines[p + 25]:
            found = True
            break  # This break won't break me out of the outer loop, which is what I really want,
    if found:      # hence the 'found' check in the outer loop
        break      # writing it all in a function would also work - using `return` to jump out of the inner loop

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha. So any and map are lazy-evaluating when working on generators.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Essentially, yeah 🦖

@Arxcis Arxcis merged commit 5783d66 into Arxcis:main Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants