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

Computational Art Project-Shreya Rangarajan #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

srangar03
Copy link

No description provided.

Copy link

@branchwelder branchwelder left a comment

Choose a reason for hiding this comment

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

There are some steps you could take towards making your code more readable (comments, breaking up conditionals, better variable names) but other than that this looks good.

pass
random_list = ["prod", "avg", "cos_pi", "sin_pi"]
random_func = random.choice(random_list)
if max_depth > 0:

Choose a reason for hiding this comment

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

This is a giant conditional. There's nothing necessarily "wrong" with nesting conditionals this deep, but I've found that once you reach three layers of if statements it's probably best to sit back and try to understand why you need to do that, because chances are you can refactor it to be more readable. Adding comments explaining what you are doing helps as well.

else:
return "x"
else:
if random.randint(0,1) == 1:

Choose a reason for hiding this comment

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

You could use return random.choice(["x", "y"]) here instead of another conditional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants