Skip to content
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

India - Water #36

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

India - Water #36

wants to merge 4 commits into from

Conversation

indiakato
Copy link

No description provided.

Copy link

@CheezItMan CheezItMan left a comment

Choose a reason for hiding this comment

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

Overall nice work India. You hit the main learning goals. Check out my comments especially regarding the reverse_inplace method.

Comment on lines +3 to 5
# Time complexity: O(n)
# Space complexity: O(n)
def factorial(n)

Choose a reason for hiding this comment

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

👍

Comment on lines 17 to 19
# Time complexity: ?
# Space complexity: ?
def reverse(s)

Choose a reason for hiding this comment

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

👍 Time & space complexity?

Comment on lines +27 to 29
# Time complexity: O(n^2)
# Space complexity: O(n^2)
def reverse_inplace(s)

Choose a reason for hiding this comment

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

⚠ This method is not reversing the string in place, instead it's creating a new string with each recursive call.

Could you see a way to do it with the following method signature?

Suggested change
# Time complexity: O(n^2)
# Space complexity: O(n^2)
def reverse_inplace(s)
# Time complexity: O(n^2)
# Space complexity: O(n^2)
def reverse_inplace(s, low = 0, high = s.length - 1)

Comment on lines +42 to 44
# Time complexity: O(n)
# Space complexity: O(n)
def bunny(n)

Choose a reason for hiding this comment

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

👍

Comment on lines +55 to 57
# Time complexity: O(n^2)
# Space complexity: O(n^2)
def search(array, value)

Choose a reason for hiding this comment

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

👍 However can you see how to do this method with O(n) space/time complexity?

return false
else
return is_palindrome(s[1..-2])
end
end

Choose a reason for hiding this comment

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

Just noting the incomplete methods.

Comment on lines +67 to 69
# Time complexity: O(n^2)
# Space complexity: O(n^2)
def is_palindrome(s)

Choose a reason for hiding this comment

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

👍 Similar note to the above on time/space complexity.

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.

2 participants