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

Karis - Edges - Palindrom Check #30

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

Conversation

kimj42
Copy link

@kimj42 kimj42 commented Oct 29, 2018

No description provided.

@@ -1,5 +1,43 @@
# A method to check if the input string is a palindrome.
# Return true if the string is a palindrome. Return false otherwise.
def palindrome_check(my_phrase)
raise NotImplementedError
return false if !my_phrase
return true if my_phrase == ""

Choose a reason for hiding this comment

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

You could combine lines 5 and 6 to be: return true if my_phrase.length < 2


until min > max
# unless my_phrase[min] == " " || my_phrase[max] == " "
until my_phrase[min] != " "

Choose a reason for hiding this comment

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

In the two nested inner loops, continue to check and confirm that min is less than max before comparing the character at the index min (or max) with a white space.

# require 'pry'; binding.pry
end

return check

Choose a reason for hiding this comment

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

If the code reaches this point, you'll always be returning true. So you could get rid of the variable check and simply return true here. Accordingly, you could simplify lines 30 through 34.

@shrutivanw
Copy link

Nice work!

I added some comments inline to make your algorithm even more robust and to simplify your code.

I didn't see any comments on the time and space complexity of your algorithm. Give it some thought. Friendly reminder: Always explain what n stands for while explaining your time and space complexity. That along with your explanation for reasoning behind your answer, is the complete answer for the time and space complexity. In this case, n will be the number of characters in the input string parameter.

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