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

beckys comments and branch #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
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
26 changes: 26 additions & 0 deletions evaluation-template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
### Project Feedback + Evaluation

* __Project Workflow__: **Meets Expectations**

>Did you complete the user stories, wireframes, task tracking, and/or ERDs, as specified above? Did you use source control as expected for the phase of the program you’re in (detailed above)?

***Great Job listing your technologies, approach and including user stories in the README!***


* __Technical Requirements__: **Exceeds Expectations**

>Did you deliver a project that met all the technical requirements? Given what the class has covered so far, did you build something that was reasonably complex?

***Your project definitely meets all of the technical requirements but also adds additional layers of complexity using the measurement bar at the bottom for example. The layout and design is great. Awesome job!***

* __Code Quality__: **Meets Expectations**

>Did you follow code style guidance and best practices covered in class, such as spacing, modularity, and semantic naming? Did you comment your code as your instructors have in class?

***You did a great job using components of OOJS, forEach Methods, functions, etc. in your code. The only additional feedback I have is to clean up your code, especially removing the console.logs. This will help for easier readability as well as identifying future errors***

* __Problem Solving__: **Exceeds Expectations**

>Are you able to defend why you implemented your solution in a certain way? Can you demonstrate that you thought through alternative implementations?

***Your project is fairly complex in a great way. I think you did a great job working through your problems and implementing not only your MVP but Silver/Gold features as well!***
2 changes: 2 additions & 0 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
<meta charset="UTF-8">
<title>I Declare War</title>
<link href='https://fonts.googleapis.com/css?family=Carrois+Gothic' rel='stylesheet' type='text/css'>
<!--Watch out here your html did not validate, you are missing a closing bracket here:
It should be <link rel="stylesheet" href="styles.css"> -->
<link rel="stylesheet" href="styles.css"
</head>
<body>
Expand Down
54 changes: 54 additions & 0 deletions project1.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// AWESOME JOB WITH YOUR COMMENTS :)
console.log('scripts up...')
//get the environment ready

// since you put your script tag at the bottom this isn't necessary:
window.addEventListener('load', function() {
game.setUpGame();
});
Expand All @@ -11,6 +14,8 @@ var computerScoreDisplay = document.querySelector('#computer-score');
var youScoreDisplay = document.querySelector('#you-score');

//event listeners / handlers

// NICE job with event listeners and functions!
var setUpTrig = document.querySelector('.reset-trig');//set/reset game on click
setUpTrig.addEventListener('click', function() {
game.setUpGame();
Expand All @@ -26,6 +31,7 @@ displayCloser.addEventListener('click', function() {
});

//game object
// LOVE that you are using OOP here :)
var game = {
warDeck: [],
players: [],
Expand All @@ -34,6 +40,12 @@ var game = {
var self = this;
values.forEach(function(val) {
suits.forEach(function(cardSuit) {
// just for readability I might instead put this string below into a variable

// For Example:
// var cardInfo = 'img/Playing_card_' + cardSuit + '_' + val + '.svg';
// self.warDeck.push({cardName: cardInfo})

self.warDeck.push(
{
cardName: 'img/Playing_card_' + cardSuit + '_' + val + '.svg',
Expand All @@ -45,6 +57,7 @@ var game = {
val.worth = self.warDeck.indexOf(val);

})
// it's definitely good to console.log things while your debugging/coding but just make sure to take the out after for readability and keep code clean
console.log('og deck: ');
console.log(self.warDeck)
},
Expand All @@ -63,6 +76,7 @@ var game = {
console.log(self.warDeck);
},
buildPlayers: function() {
// I really like your use of forEach and using objects!
var self = this;
playersToAdd.forEach(function(val) {
self.players.push({
Expand Down Expand Up @@ -111,6 +125,9 @@ var game = {

},
compareCards: function() {
// This compareCards method is really long and here is a place to refactor
// I think you could take some of your code here and put it into a separate method

var self = this;
//function to stage cards against each other in each round
//put a card from each player into stage area. card comes from the beginning of each players deck
Expand All @@ -122,6 +139,21 @@ var game = {
console.log('stage area: ');
console.log(self.stage);
//put staged cards into document body

// FOR example: below, maybe instead create a method:
// stageCards = function(self){
// var cardElComputer = document.getElementById('display-card0');
// var cardElYou = document.getElementById('display-card1');
// self.stage.forEach(function(card) {
// if (card.cardMark == 'Computer') {
// cardElComputer.src = card.cardName;
// cardElComputer.alt = card.carName;
// } else if (card.cardMark == 'You') {
// cardElYou.src = card.cardName;
// cardElYou.alt = card.cardName;
// }
// })
// }
var cardElComputer = document.getElementById('display-card0');
var cardElYou = document.getElementById('display-card1');
self.stage.forEach(function(card) {
Expand Down Expand Up @@ -171,6 +203,13 @@ var game = {
//show score in display with a display meter made from divs on game init
if(roundWinner == 'Computer') {
var compScoreUnit = document.createElement('div');
// You are using the same code as below in this if/else conditional

// I think it would be easier to put this again into a separate method
// Can you think of a way to combine this?
// Lines 210-213 and 215-219 are almost identical, and this is a place to absract a separate method and refactor
//

compScoreUnit.className = 'score-meter'
computerScoreDisplay.appendChild(compScoreUnit);
var youScoreMeter = youScoreDisplay.childNodes.length;
Expand All @@ -183,6 +222,8 @@ var game = {
computerScoreDisplay.removeChild(computerScoreDisplay.childNodes[computerScoreMeter -1]);
}
//clear stage for next round

// Just watch out again for the console.logs below here. Make sure to clean them up for readability
self.stage = [];
console.log('stage status:');
console.log(self.stage);
Expand Down Expand Up @@ -225,6 +266,18 @@ var game = {
cardElComputer.alt = 'laptop';
cardElYou.src = "https://upload.wikimedia.org/wikipedia/commons/thumb/f/fb/718smiley.svg/200px-718smiley.svg.png"
cardElYou.alt = "smiley"

// I think I would another method that you would call for the below arrays:

// this.resetGame();

// resetGame = function(){
// game.warDeck = [],
// game.players = [],
// game.stage = []
// ETC...
// }

game.warDeck = [],
game.players = [],
game.stage = [],
Expand All @@ -245,6 +298,7 @@ var game = {
})
}
}
// I think it's great that your psuedo coding, but I think it should be kept in a separate file
/*
psuedo:
game object that contains
Expand Down
2 changes: 1 addition & 1 deletion styles.css
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@

/*Nice CSS!*/
body {
width: 100%;
font-family: 'Carrois Gothic', sans-serif;
Expand Down