Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Easier solution #14

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions ch09-lists-tuples-and-dictionaries/9a-cats-with-hats
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# At the start all cats are hatless.
cats = [False]*100

# We do a loop over each step size from 1 to 100
for step in range(1,101):

# We only look at cat indices that are a multiple of our step size
for i in range(1, 100//step + 1):

# We change the hat status for each cat we loop over
cats[i*step-1] = not cats[i*step-1]
Copy link
Contributor

@somacdivad somacdivad Dec 13, 2019

Choose a reason for hiding this comment

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

I like the trick of using not to reverse the value at the index, but I'm not convinced that the way you calculate indices here is easier to read than using the % operator.

Personally, I think the following is easier to understand:

for i in range(1, 101):
    # If the index is a multiple of the step, reverse the
    # hat status of the cat at that index
    if i % step == 0:
        cats[i-1] = not cats[i-1]

Granted, that solution requires an extra line of code, but IMO it's a lot easier to digest than first computing the number of multiples of step there are between 1 and 100, then computing those multiples in the indices.

Copy link
Author

@madamak madamak Dec 14, 2019

Choose a reason for hiding this comment

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

I agree with you, the way I wrote this part is clearly not easy to read.

I have a new suggestion as I don't like the fact that we loop over each cat and test them when we can know from the start exactly which ones we need to visit.

# For each step size we calculate how many cats we are going to visit
cats_to_visit = 100 // step_size

for i in range (1, cats_to_visit + 1):
   # We reverse the hat status for each cat we visit
    cats[i*step] = not cats[i*step]

What do you think?

If you agree, I am a beginner with Github, should I directly edit my file with this?


# Print the list of cats that have a hat
for i in range(100):
if cats[i]:
print(f"Cat {i+1} has a hat!")
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 on this part of the solution. The statement of the problem says:

Write a program that simply outputs which cats have hats at the end.

I think the other solutions actually don't do a good job of this. They print a list of indices of cats that have hats.

Printing something like what you have here seems more in line with what the challenge asks for. 👍