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

Sockets - Kirsten #34

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

Sockets - Kirsten #34

wants to merge 2 commits into from

Conversation

kanderson38
Copy link

Portmanteau Generator

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
What went well in your code style, such as indentation, spacing, variable names, readability, etc.? What was lacking? I think my code style was good -- I kept track of the indentation to make it readable and added comments to explain a couple less obvious things.
How did creating the is_vowel? method affect your project? It made the code cleaner and more DRY because it wasn't necessary to write out the is_vowel? procedure more than once -- I could just call the method multiple times.
What was your strategy for getting the correct set of letters from the first word into the portmanteau? I used each_char.with_index to get the index of each vowel in the word and add it to an array; I then sliced the word at the point indicated by the final element in that array (which represented the last vowel in the word).
What was your strategy for validating against inputs under 2 characters? I used an until loop to check the length of the user input; if the input had fewer than two letters, the loop would run and ask the user to reenter their word.
In the next project, what would you change about your process? What would you keep doing? I would probably write more pseudocode during my process to make sure I had a good handle on the algorithms I was trying to write (figuring out how to find the last vowel in the word took a while, because I was trying to do all the work in my head). Otherwise, I think my process was good, and I'll keep following recommended practices (indentation, comments, variable names that make sense) to make my code readable.

@dHelmgren
Copy link

Portmanteau Generator

What We're Looking For

Feature Feedback
Readable code with consistent indentation Yes
Practices using variables appropriately yes
Practices using conditionals appropriately Yes
Practices iteration appropriately Generally good, see comments!
Practices using custom methods appropriately yes
Program validates against input under 2 characters Yes
Takes in two inputs and creates a portmanteau yes!

end

continue = validate(word)
if continue == false

Choose a reason for hiding this comment

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

could you reduce this loop structure so that you check both the word length and the vowel count? Perhaps with elsif?

if is_vowel?(temp_array[i])
vowel_array << i
end
end

Choose a reason for hiding this comment

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

While effective, this solution is doing a lot more work than it needs to. It will take the program less time to run if you just returned the index of the first or last value in the word.

print "Would you like to make another portmanteau (y/n)? "
run_again = gets.chomp
while run_again != "y" && run_again != "n"
print "Please enter \"y\" for yes or \"n\" for no. "

Choose a reason for hiding this comment

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

This portion never runs because run_again starts by being set to 'y', so this while is always skipped


def is_vowel?(letter)
case letter
when "a", "e", "i", "o", "u", "A", "E", "I", "O", "U"

Choose a reason for hiding this comment

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

good job catching upper case! .downcase would also achieve this effect.

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