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

Implement get_letter_grade and shortest_string #106

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sheshtawy
Copy link

No description provided.

@sheshtawy
Copy link
Author

@jaybobo

Copy link
Member

@jaybobo jaybobo left a comment

Choose a reason for hiding this comment

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

looks good but see comments.


#Put your code here!
grade = ''
case integer
Copy link
Member

Choose a reason for hiding this comment

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

the case statement returns whatever the result of whatever meets the condition so no need to use the grade variable anywhere within this function.

see: https://ruby-doc.org/docs/keywords/1.9/Object.html#method-i-case


return grade
Copy link
Member

Choose a reason for hiding this comment

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

ruby functions will return the last evaluated expression so explicit returns aren't used often except in the case of guard clauses.

example where someone might use an explicit return.

def is_tall?(person)
  return false if person.short?
  true
end

end

def shortest_string(array)

#Put your code here!
sorted_array = array.sort_by { |word| word.length }
Copy link
Member

Choose a reason for hiding this comment

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

this can also be written a bit more tersely but still readable. see: https://ruby-doc.org/core-2.5.1/Enumerable.html#method-i-min_by

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