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

Project1 update #36

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

Project1 update #36

wants to merge 18 commits into from

Conversation

G174
Copy link

@G174 G174 commented Sep 30, 2017

No description provided.

var submitAnswer = ""

var country =["vietnam", "china", "italy", "mexico", "india"]
var image = ["url(./assets/img/vietnam.png)","url(./assets/img/china.png)",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't you store just the string of the image instead?

@shumin13
Copy link

Glow

  • Educational game
  • Clear comments

Grow & Things to look out for

  • Can look into adding more levels
  • Need to work on commit messages
  • Good to hide skip and submit button before the start button is clicked
  • The background size changed slightly after the start button is clicked.
  • To reset the text of the score to 0 after the user click on try again

// console.log(usedIndex)
randomIndex = Math.floor(Math.random()*5)
if (usedIndex.length < 5){
while (usedIndex.includes(randomIndex)) {

Choose a reason for hiding this comment

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

incorrect indentation

function shuffleImg() {
// console.log(usedIndex)
randomIndex = Math.floor(Math.random()*5)
if (usedIndex.length < 5){
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain the loop more here

timerObj.startTimer()
if (randomIndex === 0) {
$('<input type="text" class="input" placeholder="* * * * * * *">').insertBefore(".submit");
} else if (randomIndex === 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the randomIndex variable are for

$('.submit').click(function() {
var $input = $('.input')
submitAnswer = $input.val()
checkAnswer()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't you pass the answer into the checkAnswer function


//does submitAnswer match answer
function checkAnswer() {
if(submitAnswer === answer && timerObj.seconds < 5) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we not repeat these all conditions?

@primaulia
Copy link
Contributor

Project Workflow: 3 / 5
Technical Requirement: 3 / 5
Creativity: 4 / 5
Code Quality: 3 / 5
Problem Solving: 4 / 5
Delivery: 4 / 5
Professional Skill: 4 / 5

Glow

  • Good job scaling down the project to your advantage. I think the game is pretty well done in terms of design, logic, and complexity
  • I love the game idea, definitely something that you could improve more for the meet and greet
  • Workflow is easy to follow and focus on important points

Grow

  • I think the workflow section of your readme should start with flowchart, to make it easy for other people to understand
  • A couple of repetitions that can be rectified easily and make your game more interesting with many levels
  • The usage of function can be more DRY here, so you dont have to keep creating different function for different scenarios

Things to look out for

  • Fundamental understanding of logical flow, keep practicing with codewars and exercism

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