Skip to content

Update 3n+1.py - #996

Merged
cclauss merged 4 commits into
TheAlgorithms:masterfrom
SandersLin:patch-18
Jul 13, 2019
Merged

Update 3n+1.py#996
cclauss merged 4 commits into
TheAlgorithms:masterfrom
SandersLin:patch-18

Conversation

@SandersLin

Copy link
Copy Markdown
Contributor

Made variable names more meaningful and removed nested functions.

Made variable names more meaningful and removed nested functions.
Comment threadmaths/3n+1.py Outdated
@cclauss

Copy link
Copy Markdown
Member

What should happen on a negative number? a floating point number?

defn31(initial_number):
""" >>> n31(43) # doctest: +ELLIPSIS ([43, ...], 29) """

@SandersLin

Copy link
Copy Markdown
ContributorAuthor

Good catch! Added argument validation and type hints.

Comment threadmaths/3n+1.py Outdated
counter += 1
path += [a]
return path, counter
return path, counter + 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it also work to return path, len(path) ?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, that is more elegant, thanks!

Comment threadmaths/3n+1.py Outdated
"""
Returns Collatz sequence of number a

Returns Collatz sequence of a number

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This comment says that we are only returning the sequence but we are returning the sequence and its length.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

updated docstring

Comment threadmaths/3n+1.py
def main():
num = 4
path , length = n31(num)
print("The Collatz sequence of {0} took {1} steps. \nPath: {2}".format(num,length, path))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can drop the 0, 1, and 2 here and drop the 0 in the raise statements above.

@cclauss
cclauss self-requested a review July 13, 2019 07:04

@cclausscclauss left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's ship it.

@cclauss
cclauss merged commit 1e0b33d into TheAlgorithms:masterJul 13, 2019
stokhos pushed a commit to stokhos/Python that referenced this pull request Jan 3, 2021
* Update 3n+1.py
Made variable names more meaningful and removed nested functions.
* Update 3n+1.py
* Update 3n+1.py
* Update 3n+1.py
Sign up for freeto 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

@SandersLin@cclauss