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 - Cyndi #32

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
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
111 changes: 111 additions & 0 deletions generator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
def is_vowel(letter)
case letter
when "a", "e", "i", "o", "u"
return "true"
else
return "false"

Choose a reason for hiding this comment

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

You should return true or false (the boolean value) here, rather than "true" or "false" (a string containing the word "true" or "false"). Getting comfortable with using different data types is an important thing to practice early on.

end
end

def rev_str(strg)
aa = strg.split("")
rev = Array.new
aa.length.times do

Choose a reason for hiding this comment

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

Could you use more specific variable names here?

  • rev_str could be reverse_string
  • aa could be string_array
  • rev could be reversed

IMO, that would make this code much easier to read.

Choose a reason for hiding this comment

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

Also, Ruby has a way to do this built in: String#reverse

puts "abcd".reverse
# => dcba

rev << aa.pop
end
rev = rev.join
return rev
end

def check_length(word)
if word.length < 2
until word.length > 1
puts "Your word must be 2 or more characters."
word = gets.chomp

Choose a reason for hiding this comment

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

I think this method would do exactly the same thing if you removed the if statement entirely:

def check_length(word)
  until word.length > 1
    puts "Your word must be 2 or more characters."
    word = gets.chomp
  end
  return word
end

end
return word
else
return word
end
end

def vowel_index(word)
count = 0
word.each_char { |letter|
if is_vowel(letter) == "false"

Choose a reason for hiding this comment

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

In general, I would prefer you use do..end over {..}. It makes the code a little easier to read.

count += 1
else
break
end
}
return count
end

def check_contain_vowel(word)
count = vowel_index(word)
if count == word.length
until count != word.length

Choose a reason for hiding this comment

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

Again, you don't need both the if and the until, since they're checking the same thing. If count != word.length, the until loop won't run at all.

puts "Word must contain a vowel."
word = gets.chomp
count = vowel_index(word)
end
end
return word
end

def run_generator
#----get first and second word from caller----
puts "What is the first word to combine?"
a = gets.chomp.downcase
a = check_length(a)
a = check_contain_vowel(a)
# get second word
puts "What is the second word to combine?"
b = gets.chomp.downcase
b = check_length(b)
b = check_contain_vowel(b)

Choose a reason for hiding this comment

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

I love the idea of breaking these individual tasks out into separate methods - that makes the flow of this code very easy to understand. Good work!

If you wanted to go even further, you could combine these 4 repeated lines into another method, maybe something like get_user_input.


#----find vowel indices
#first word
rev = rev_str(a) #reverse string
count1 = vowel_index(rev)
#since word was reversed, find index of original word
index1 = a.length - count1 - 1

#second word
count2 = vowel_index(b)
index2 = count2

#----splice words according to rules
#first word
#if no vowels found then print out entire word, otherwise extract substrings
if count1 == a.length
firstword = a
else
firstword = a.slice(0...index1)
end
#second word
#if no vowels found then print out entire word, otherwise extract substrings
if count2 == b.length
secondword = b
else
secondword = b.slice(index2..b.length)
end

#----combine words
word = firstword + secondword
#print out combined word
puts word
end

run_generator
rerun = nil
until rerun == "n" || rerun == "no"
puts "Would you like to re-run this program? Write 'Y' for yes and 'N' for no."
rerun = gets.chomp.downcase
case rerun
when "yes", "y"
run_generator
else
end
end