jblakeman · joe-gz · Feb 29, 2016 · Feb 29, 2016
diff --git a/feedback_jgz.md b/feedback_jgz.md
@@ -0,0 +1,25 @@
+# Project Feedback
+
+## Project Workflow
+
+>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)?
+
+**Meets Expectations**. I love to see such an extensive commit history! That's super important, and will really be handy when you get into group work too. Would love to see user stories added in next time too!
+
+## Technical Requirements
+
+>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?
+
+**Meets Expectations**. Your project shows a great understanding of the concepts and materials we've covered so far! I love that you've taken a creative approach and created a game different from the original memory game. Next step would be to attack those things to add that you listed in the readme!
+
+## Code Quality
+
+>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?
+
+**Exceeds Expectations**.  Please review my inline comments (prefixed with my initials - "JGZ") for suggestions to help improve code quality.  My biggest piece of feedback here is to make sure you're commenting some of the larger functions.  For others who might go in and check out your code (especially working in groups), commenting is so important. Other than that, the code is very clean, neatly indented, and passes the validators.  I left notes with a few questions in line.  As far as your question regarding using an object, i think that was a great way to approach this! I understand it can get a little confusing with the "self" and "this". My suggestion would be to console log those often to make sure you're targeting the right thing.
+
+## Problem Solving
+
+>Are you able to defend why you implemented your solution in a certain way? Can you demonstrate that you thought through alternative implementations?
+
+**Exceeds Expectations** You're commit history shows you approached this in a great way! Taking it step by step, and then cleaning it up once you have something functional. It would be great to see some evidence of a branch trying to add in any extra features, such as a reset button, scoreboard, etc.
diff --git a/index.html b/index.html
@@ -1,3 +1,6 @@
+<!-- JGZ - No validator errors, nice!!
+I love how minimal and clean your html is here -->
+
 <!DOCTYPE html>
 <html>
   <head>

diff --git a/memory_grid.js b/memory_grid.js
@@ -1,3 +1,6 @@
+// JGZ - Would love to see some in line commenting! there are some fairly complicated functions here
+// So it would be super useful for people who wanted to check out you code to see some comments explaining what you are doing
+
 var memory = {
     grid: {
         height: 6,
@@ -20,20 +23,28 @@ var memory = {
         correct: 0
     },
     hideAfterMs: 3350,
+    // JGZ - I love this function here! Would this be to eventually allow the user to select the size of the board?
     setNumFilled: function() {
         this.grid.fillNum = Math.floor((this.grid.height*this.grid.width)/3)+1;
     },
     createNewElement: function(tagName, className, idName) {
         var newElement = document.createElement(tagName);
+    // JGZ - I totally get that this is a stylistic choice, so feel free to disagree, but I'd recommend using "{ }"'s even for one line conditionals
+    // The reason being that it can be tough for others to read your code and understand exactly where the conditional ends.
+    // More importantly, moving forward it could lead to some confusion if you eve need to expand such conditionals. I'd just think about it as best practice!
         if (className) newElement.className = className;
         if (idName) newElement.id = idName;
         return newElement;
     },
     createGrid: function() {
+      // JGZ - interesting approach to creating variables here.
+      // I'm curious as to why, instead of simply declaring them when they arise? there don't seem to be any scoping issues..
         var row, rowElement, cell, i, j;
         var table = document.getElementById("grid");
         this.setNumFilled();
+        // JGZ - watch indentation here!
 		for (i = 0; i < this.grid.height; i++) {
+      // JGZ - COOL! i had no idea you could add classes like this when creating an element!
             rowElement = this.createNewElement("tr", "row");
             row = [];
             this.grid.empty[i] = {};
@@ -48,6 +59,8 @@ var memory = {
         }
     },
     populateRandomGrid: function(self) {
+      console.log(self);
+      // JGZ - I consider more semantic naming than r1,and r2?
         var numAssigned = 0, r1, r2;
         self.grid.filled = {};
         while(numAssigned < self.grid.fillNum) {
@@ -57,7 +70,7 @@ var memory = {
                 r2 = Math.floor(Math.random() * self.grid.width);
                 if (!(r2 in self.grid.filled[r1])) {
                     self.grid.filled[r1][r2] = false;
-                    self.grid.elements[r1][r2].style.backgroundColor = 
+                    self.grid.elements[r1][r2].style.backgroundColor =
                         self.grid.colors.filled;
                     numAssigned++;
                 }
@@ -70,6 +83,8 @@ var memory = {
         if (self.grid.fillNum === self.guesses.used) {
             rateCorrect = self.guesses.correct + "/" + self.guesses.used;
             self.statusBar.innerText = rateCorrect + " correct";
+            // JGZ - I'd consider passing in the below numbers as arguments to a function and pulling this statement out!
+            // I personally like to avoid nested if's if at all possible, just for clarity.
             if (self.guesses.correct === self.grid.fillNum) {
                 doneMsg = "Perfect Game!";
             } else {
@@ -94,7 +109,7 @@ var memory = {
             self.grid.elements.forEach(function(row, i1) {
                 row.forEach(function(cell, i2) {
                     var color = self.grid.colors.miss;
-                    var correct = ((i1 in self.grid.filled) && 
+                    var correct = ((i1 in self.grid.filled) &&
                                    (i2 in self.grid.filled[i1]));
                     if (correct) color = self.grid.colors.hit;
                     cell.addEventListener("click", function(event) {
@@ -110,6 +125,7 @@ var memory = {
                             self.statusBar.innerText = (self.grid.fillNum -
                                                         self.guesses.used) +
                                                        " Guesses left";
+                    // JGZ - could you not just pass in self here instead of the event?
                             self.changeBackgroundColor(event, color);
                             self.checkForWinner(self);
                         }

diff --git a/style.css b/style.css
@@ -1,3 +1,8 @@
+ /*JGZ - No validator errors, nice!!*/
+ /*JGZ - This is super clean, well done! I really like that you've combined selctors with the same styling into on selection*/
+ /*JGZ - My next challenge would be to srart playing arounf with "em" and "%" for things like width, height, and font size.
+ This will help you add in some responsiveness to you app on window size changes!*/
+
 body * {
     font-family: "Russo One", "sans-serif";
 }
@@ -56,4 +61,3 @@ td.cell {
 #start-new {
     margin-bottom: 15px;
 }
-