Skip to content

Conversation

@parth-paradkar
Copy link
Contributor

@parth-paradkar parth-paradkar commented Jun 27, 2019

Hey! New to contributing so sorry for any errors in committing.
Added the Tau constant which is the ratio of a circle's circumference to its radius (2 * Pi)
Added the factorial function that calculates the factorial of an integer

Value of Tau constant added

Factorial function added, Tau constant added

Factorial function added

Factorial function made public

Code cleaned
@medvednikov
Copy link
Member

Hey

None of major languages have factorials in the stdlib. Tau is fine.

@parth-paradkar
Copy link
Contributor Author

Python does have a factorial function in its math module. (math module docs)
But that's cool... Should I add a commit with the factorial function removed?

@medvednikov
Copy link
Member

Indeed it does. Ok, can you make it return i64 and use for ;; loop?

@parth-paradkar
Copy link
Contributor Author

Done!

@medvednikov medvednikov merged commit 5651ba5 into vlang:master Jun 27, 2019
pub fn factorial(a int) i64 {
mut prod := 1
for i:= 0; i < a; i++ {
prod = prod * (i+1)
Copy link
Member

Choose a reason for hiding this comment

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

@thescriptninja prod *= i+1

Copy link
Contributor

Choose a reason for hiding this comment

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

Using i := 1 would be more efficient.

Copy link
Contributor

@ntrel ntrel left a comment

Choose a reason for hiding this comment

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

This should panic if a! is bigger than i64.max.

pub fn factorial(a int) i64 {
mut prod := 1
for i:= 0; i < a; i++ {
prod = prod * (i+1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Using i := 1 would be more efficient.

@ntrel
Copy link
Contributor

ntrel commented Jun 28, 2019

@medvednikov maybe leave pulls open a bit longer for reviews ;-)

Copy link
Contributor

@ntrel ntrel left a comment

Choose a reason for hiding this comment

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

This also needs to panic when a < 0.

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.

4 participants