PR-2 by CheezItMan · Pull Request #2 · Ada-C12/code-reviews

CheezItMan · 2020-02-06T18:50:37Z

This method calculates a factorial

brilatimer · 2020-02-06T21:48:02Z

function.rb

+
+def function(n)
+  if n == 0
+    1


could be clearer

dtaylor73 · 2020-02-06T21:42:02Z

function.rb

@@ -0,0 +1,9 @@
+
+def function(n)


Let's make this function dryer. We could write a ternary statement instead of spreading the code out into multiple lines.

dtaylor73 · 2020-02-06T21:44:17Z

function.rb

+
+def function(n)
+  if n == 0
+    1


Let's write 'puts' in front of the code we want to print to the screen.

jaitch · 2020-02-06T21:40:37Z

function.rb

@@ -0,0 +1,9 @@
+
+def function(n)
+  if n == 0


Did you mean to include "return" before the "1"?

Galaxylaughing

ok

Galaxylaughing · 2020-02-06T21:44:03Z

function.rb

@@ -0,0 +1,9 @@
+
+def function(n)


function doesn't seem like a meaningful name; it should probably be called 'factorial' or something

Galaxylaughing · 2020-02-06T21:44:45Z

function.rb

+
+def function(n)
+  if n == 0
+    1


I think you need to explicitly return 1 here and, on line 6, return n * function(n-1)

Galaxylaughing · 2020-02-06T21:45:28Z

function.rb

@@ -0,0 +1,9 @@
+
+def function(n)
+  if n == 0


if n is a float, this function might go on forever since it will never == zero. Perhaps <= would be better?

raisahv

Approved

raisahv · 2020-02-06T21:43:45Z

function.rb

+    1
+  else
+    n * function(n-1)
+  end


can shorten this to

Suggested change

end

n == 0 ? 1 : n * function(n-1)

dtingg · 2020-02-06T21:46:41Z

function.rb

@@ -0,0 +1,9 @@
+
+def function(n)


Maybe rename this function to factorial?

dtingg · 2020-02-06T21:47:07Z

function.rb

+  if n == 0
+    1
+  else
+    n * function(n-1)


If you change the name of the function, you have to change it here too.

north108 · 2020-02-06T21:44:11Z

function.rb

@@ -0,0 +1,9 @@
+
+def function(n)


function naming should be more clear as to what it does

north108 · 2020-02-06T21:44:48Z

function.rb

+
+def function(n)
+  if n == 0
+    1


add return to be more explicit

sun-alice · 2020-02-06T21:44:18Z

function.rb

@@ -0,0 +1,9 @@
+
+def function(n)


name this something more elaborate

sun-alice · 2020-02-06T21:46:20Z

function.rb

+  if n == 0
+    1
+  else
+    n * function(n-1)


Suggested change

n * function(n-1)

return n * function(n-1)

sun-alice · 2020-02-06T22:15:38Z

function.rb

+
+def function(n)
+  if n == 0
+    1


Suggested change

1

return 1

emilyvomacka · 2020-02-06T21:42:44Z

function.rb

+  if n == 0
+    1
+  else
+    n * function(n-1)


Should handle the initial case of n < 0

emilyvomacka · 2020-02-06T21:51:40Z

function.rb

+
+def function(n)
+  if n == 0
+    1


could add "return" to make it clearer

mdove92 · 2020-02-06T21:48:31Z

function.rb

+
+def function(n)
+  if n == 0
+    1


What is the 1 supposed to be set to? Are you trying to return it? Please write it out

mdove92 · 2020-02-06T21:49:11Z

function.rb

@@ -0,0 +1,9 @@
+
+def function(n)


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.

function method added

1ecb4a0

brilatimer reviewed Feb 6, 2020

View reviewed changes

function.rb

def function(n)

if n == 0

1

Copy link

brilatimer Feb 6, 2020

Choose a reason for hiding this comment

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

could be clearer

dtaylor73 reviewed Feb 6, 2020

View reviewed changes

jaitch reviewed Feb 6, 2020

View reviewed changes

function.rb

@@ -0,0 +1,9 @@

def function(n)

if n == 0

Copy link

jaitch Feb 6, 2020

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"?

Galaxylaughing reviewed Feb 6, 2020

View reviewed changes

raisahv reviewed Feb 6, 2020

View reviewed changes

function.rb

1

else

n * function(n-1)

end

Copy link

raisahv Feb 6, 2020

Choose a reason for hiding this comment

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

can shorten this to

Suggested change

end

n == 0 ? 1 : n * function(n-1)

dtingg reviewed Feb 6, 2020

View reviewed changes

north108 reviewed Feb 6, 2020

View reviewed changes

sun-alice reviewed Feb 6, 2020

View reviewed changes

emilyvomacka reviewed Feb 6, 2020

View reviewed changes

mdove92 reviewed Feb 6, 2020

View reviewed changes

Conversation

CheezItMan commented Feb 6, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Galaxylaughing left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raisahv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants