Skip to content

fibonacci function first implementation (missing autotest code) #11

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.

Sign up for GitHub

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

Open
DurvalMenezes wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fibonacci function first implementation (missing autotest code) #11

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2226f06
fibonacci function first implementation (missing autotest code)
DurvalMenezes Sep 24, 2024
5b9e2f4
reformatted with `gofmt`
DurvalMenezes Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions operations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ type (
opmapType map[string]ophandler
)

func fib(x uint64) uint64 {
if x < 1 || x > 45 {
// less than one is outside the domain of the mathematical function,
// and more than 45 overflows uint64 and results in infinite recursion
panic("fib() called with too small or large value!")
Copy link
Owner

@marcopaganini marcopaganini Sep 24, 2024

Choose a reason for hiding this comment

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

Panic will crash the program with a huge and ugly stack trace. It's never the best solution. If we want to quit the program with a fatal condition, log.Fatal is the way to go. Here, I think this should return an error that is then "bubbled up" to the caller and displayed to the user.

}
if x <= 2 {
return 1
}
return fib(x-1) + fib(x-2)
}

func newOpsType(stack *stackType) *opsType {
bold := color.New(color.Bold).SprintFunc()

Expand Down Expand Up @@ -112,6 +124,9 @@ func newOpsType(stack *stackType) *opsType {

ophandler{"fac", "Calculate factorial of x", 1, func(a []float64) ([]float64, int, error) {
x := uint64(a[0])
if float64(x) != a[0] {
Copy link
Owner

@marcopaganini marcopaganini Sep 24, 2024

Choose a reason for hiding this comment

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

Instead of doing this maneuver with uint64 and float to make sure the number is an integer, maybe use if math.Floor(a) != a { ... } like elsewhere in the code. This avoids the confusing type casting.

return nil, 1, errors.New("factorial requires an integer number")
}
if x <= 0 {
return nil, 1, errors.New("factorial requires a positive number")
}
Expand All @@ -121,6 +136,21 @@ func newOpsType(stack *stackType) *opsType {
}
return []float64{float64(fact)}, 1, nil
}},

ophandler{"fib", "Calculate fibonacci of x", 1, func(a []float64) ([]float64, int, error) {
Copy link
Owner

@marcopaganini marcopaganini Sep 24, 2024

Choose a reason for hiding this comment

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

Why not design fib() in a way that it can be called directly from the ophandler?

x := a[0]
if x-float64(uint64(x)) != 0 {
Copy link
Owner

@marcopaganini marcopaganini Sep 24, 2024

Choose a reason for hiding this comment

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

Maybe use the same math.Floor approach here?

Copy link
Owner

@marcopaganini marcopaganini Sep 24, 2024

Choose a reason for hiding this comment

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

With a bunch of ifs, we can use a switch. It's cleaner. Remember that in Go, a case is not restricted to one variable. You can do:

switch {
  case any_condition:
    (...)
  case any_other_condition:
    (...)
}

and so on.

return nil, 1, errors.New("fibonacci requires an integer number")
}
if x < 1. {
Copy link
Owner

@marcopaganini marcopaganini Sep 24, 2024

Choose a reason for hiding this comment

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

You don't need a dot at the end in Go. if x < 1 works fine :)

return nil, 1, errors.New("fibonacci requires a positive number")
}
if x > 45. {
Copy link
Owner

@marcopaganini marcopaganini Sep 24, 2024

Choose a reason for hiding this comment

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

Same here (dot)

return nil, 1, errors.New("fibonacci can currently only be computed for numbers smaller than 45")
}
return []float64{float64(fib(uint64(x)))}, 1, nil
}},

"",
bold("Bitwise Operations"),
ophandler{"and", "Logical AND between x and y", 2, func(a []float64) ([]float64, int, error) {
Expand Down