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

Maryam's reverse words #27

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

Maryam's reverse words #27

wants to merge 4 commits into from

Conversation

marshi23
Copy link

No description provided.

Copy link

@kangazoom kangazoom left a comment

Choose a reason for hiding this comment

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

we solved this one in different ways, but i think yours is very clever how you use nil, true, and false values to keep track. i wouldn't have thought of it, and i feel like i learned something new!

finish = nil

while current_index < my_words.length
currently_on_letter = my_words[current_index] != " "

Choose a reason for hiding this comment

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

i haven't seen this before - is it just returning true or false?

end

if start && finish
reverse_one_word(my_words, start, finish)

Choose a reason for hiding this comment

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

i like that you split it into different functions - easy to follow

first_letter = word[i]
last_letter = word[index]

word[i] = last_letter

Choose a reason for hiding this comment

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

Idea for further improvement: can you swap using only one temporary variable instead of two as you have now?

index -= 1
end

return word

Choose a reason for hiding this comment

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

Since you're reversing in place, you don't need to return word. Line 23 could simply be return

@shrutivanw
Copy link

Nice work! 👍

I didn't see any comments or description on the time and space complexity. Take a read of the explanations on https://github.com/Ada-C10/reverse_words/blob/solution/lib/reverse_words.rb

Slack me if you have any questions.

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.

3 participants