samir-acle · RobertAKARobin · Nov 2, 2015
diff --git a/css/styles.css b/css/styles.css
@@ -1,3 +1,4 @@
+/* Validates! */
 *
 {
     box-sizing: border-box;

diff --git a/evaluation.md b/evaluation.md
@@ -0,0 +1,25 @@
+# Project Feedback + Evaluation
+
+## 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)?
+
+**Exceeds expectations**: Great readme! Watch out for your commit messages -- make sure they're all descriptive. If you find yourself writing a nondescriptive commit message, you probably don't need that commit. For example: commit `da8e06d` is "restructured code". While we encourage commit early and commit often, once you get the hang of that it's a good idea to instead commit *intentionally*. I make a lot of use of `git reset --soft head~` to "undo" previous commits so I can add more stuff in. 
+
+## 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?
+
+**Exceeds expectations**: This clearly demonstrates a grasp of event listeners, jQuery selectors, and DOM traversal. It may be worth considering whether you're using *too* much in this project. From an experimentation standpoint, it's awesome! If this is something you intend to publish, though (and I say go for it!) it may be overkill that adds rather a lot of bulk. Maybe not, but personally I prefer the more-with-less approach.
+
+## 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?
+
+**Meets expectations**: You have a great number of IDs and classes in your HTML, which may not be necessary. Your Javascript is all semantically-named. However, there's an awful lot of it. This makes it pretty tough to maintain. I'd encourage you to work on consolidating all of your functions and variables into a handful of objects, and then dividing those into separate files. The essence of object-oriented programming is having independent objects that clearly "pass" data and functionality between each other, rather than a single monolithic script or object.
+
+## 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**: Great stuff, Samir! Again, I'd encourage you to keep working on this as a side project and then publish it, even if just to `reddit.com/r/internetisbeautiful`. 
diff --git a/index.html b/index.html
@@ -11,6 +11,7 @@
 </head>
 <body>
 	<main>
+		<!-- Almost every element on this page has an ID and/or a class. Are they all necessary? -->
 		<div class='hud'>
 			<p class='fk-count counters'>Fully Know</p>
 			<p class='kk-count counters'>Kinda Know</p>

diff --git a/js/app.js b/js/app.js
@@ -98,6 +98,7 @@ $('.flip').on('click', function(evt) {
   flipCard();
   updateDisplay();
 });
+// Even if your <script> is at the end of your body, it's still considered good practice to put the procedural part of the script inside a $(document).ready
 
 /** click handler for keypresses*/
 $(document).on('keyup', function(evt) {
@@ -177,6 +178,31 @@ $('.dont-know').on('click', function() {
   updateDisplay();
 });
 
+/* Lots of event listeners! Could you attach them to the FlashCard object? For instance:
+
+Flashcard.events = {
+  fullyKnow: function(){
+
+  }
+}
+
+...and then:
+(function addEventListeners(){
+  $(".fully-know").on("click", Flashcard.events.fullyKnow);
+}())
+
+Or, if you want to get fancy:
+var buttons = ["fully-know", "kinda-know", "dont-know"]
+var i;
+for(i = 0; i < buttons.length; i++){
+  $("." + buttons[i]).on("click", function(){
+    eval("Game.currentCard.setAs" + buttons[i] + "()");
+    updateArrays();
+    updateDisplay();
+  });
+}
+*/
+
 /** click handler to create deck of cards*/
 $('.generate').on('click', function(evt) {
   evt.preventDefault();
@@ -220,6 +246,7 @@ $('.reset').on('click', resetGame);
 
 /** @desc creates placeholder flashcards */
 function initCards() {
+  // Would use a loop here
   Model.flashcards.push(new FlashCard('teacher',
     'lao shi'));
   Model.flashcards.push(new FlashCard('lawyer', 'lu shi'));
@@ -272,6 +299,11 @@ function updateDisplay() {
  * @desc sets boolean to opposite to represent either front or back of the card
  */
 function flipCard() {
+  /* To avoid having to indent too many levels, I'd prefer
+  if(Game.deckEmpty) return;
+  Game.onQuestion = !Game.onQuestion;
+  stopTime()...
+  */
   if (!Game.deckEmpty) {
     Game.onQuestion = !Game.onQuestion;
     stopTime();
@@ -313,6 +345,8 @@ function stopTime() {
   Game.currentCard.updateTime();
 }
 
+// To my mind, there are maybe 3 main models acting here: the flashcard, the game, and the player. Could you make all of these functions attached to those models so there are no exposed functions?
+
 /** @desc switches current flash card*/
 function nextCard() {
   if (!Game.deckEmpty) {
@@ -536,6 +570,7 @@ function generateDeck() {
     Game.deckEmpty = false;
   }
 }
+// Generally, if a file of code is more than 100 lines long, it should be split into separate files. Use Grunt or whichever package manager to take care of all the minification for you.
 
 /** @desc changes currently displayed card to the first in the deck*/
 function resetDeck() {