Skip to content

Problem Set JS Basics #7

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

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

Problem Set JS Basics #7

wants to merge 5 commits into from

Conversation

SwittS
Copy link

@SwittS SwittS commented Jul 1, 2016

At the start of the problem set I felt discouraged because I wasn't clearly understanding the logic. I was struggling mostly with logic. However, staying late after class has been very helpful with receiving assistance from other classmates. "Thinking like a programmer" has been a the most challenging part for me because it's a new experience to me. Nevertheless, I have found that pseudocode is the most helpful method to approach each problem. I will continue to review the homework readings but I feel that creating code will help me the most.

@melicarls
Copy link
Contributor

melicarls commented Jul 6, 2016

Great work! Your solutions for all of the problems meet the requirements and look great. I'm glad that working with peers is helpful. Some quick notes:

  • Bottles of Beer: Your second to last verse has a small grammar issue: "one bottle_s_ of beer on the wall." What kind of logic could you add to fix that and meet the stretch goals? Another stretch: how could you allow the starting number to vary based on a number that is passed into the function?
  • Shakespearian Insults: you can (and should) declare your rand, rand2, rand3 within the function instead of the in the global scope. In this case, they won't need to be accessed outside of your function so it's considered a better practice to encapsulate them within the function. Also, the function declaration should read var generateInsult = function() { or function generateInsult(){

@melicarls melicarls closed this Jul 6, 2016
@melicarls melicarls reopened this Jul 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants