-
Notifications
You must be signed in to change notification settings - Fork 2
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
PR-2 #2
base: master
Are you sure you want to change the base?
PR-2 #2
Conversation
|
||
def function(n) | ||
if n == 0 | ||
1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be clearer
@@ -0,0 +1,9 @@ | |||
|
|||
def function(n) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make this function dryer. We could write a ternary statement instead of spreading the code out into multiple lines.
|
||
def function(n) | ||
if n == 0 | ||
1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's write 'puts' in front of the code we want to print to the screen.
@@ -0,0 +1,9 @@ | |||
|
|||
def function(n) | |||
if n == 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to include "return" before the "1"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
@@ -0,0 +1,9 @@ | |||
|
|||
def function(n) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function
doesn't seem like a meaningful name; it should probably be called 'factorial' or something
|
||
def function(n) | ||
if n == 0 | ||
1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to explicitly return 1 here and, on line 6, return n * function(n-1)
@@ -0,0 +1,9 @@ | |||
|
|||
def function(n) | |||
if n == 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if n is a float, this function might go on forever since it will never == zero. Perhaps <= would be better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved
1 | ||
else | ||
n * function(n-1) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can shorten this to
end | |
n == 0 ? 1 : n * function(n-1) |
@@ -0,0 +1,9 @@ | |||
|
|||
def function(n) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe rename this function to factorial?
if n == 0 | ||
1 | ||
else | ||
n * function(n-1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you change the name of the function, you have to change it here too.
@@ -0,0 +1,9 @@ | |||
|
|||
def function(n) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function naming should be more clear as to what it does
|
||
def function(n) | ||
if n == 0 | ||
1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add return to be more explicit
@@ -0,0 +1,9 @@ | |||
|
|||
def function(n) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name this something more elaborate
if n == 0 | ||
1 | ||
else | ||
n * function(n-1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
n * function(n-1) | |
return n * function(n-1) |
|
||
def function(n) | ||
if n == 0 | ||
1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 | |
return 1 |
if n == 0 | ||
1 | ||
else | ||
n * function(n-1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should handle the initial case of n < 0
|
||
def function(n) | ||
if n == 0 | ||
1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could add "return" to make it clearer
|
||
def function(n) | ||
if n == 0 | ||
1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the 1 supposed to be set to? Are you trying to return it? Please write it out
@@ -0,0 +1,9 @@ | |||
|
|||
def function(n) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should have a more descriptive name. Function dos not let us know what to use it for, and may be a reserved word in ruby.
This method calculates a factorial