From f63b09ef3123b43c9963d3ceb503bb47c4976c93 Mon Sep 17 00:00:00 2001
From: Aaron Krause <aaroncrows@Aarons-MacBook-Pro.local>
Date: Fri, 16 Sep 2016 09:29:38 -0700
Subject: [PATCH] comments on final project

---
 about-info.js | 10 +++++++++-
 game.js       |  8 ++++++++
 index.html    |  2 ++
 style.css     | 19 ++++++++++++++++++-
 4 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/about-info.js b/about-info.js
index 292a760..5331962 100644
--- a/about-info.js
+++ b/about-info.js
@@ -5,12 +5,15 @@ var sortedHighScores;
 var arrayOfScores;
 var scoreStrings = ['high_score_easy','high_score_med','high_score_hard'];
 
+//this might not have needed to be a function. Since it's only used once
+//and it's a relatively simple operation.
 function getScores() {
   var arrayString;
   arrayString = localStorage.getItem('high_score_array');
   return JSON.parse(arrayString);
 };
-
+//this function is only used once and is effectively calling the sort method
+//on an array once. You could just call sort on the array below.
 function objectSort(array) {
   var sorted;
   sorted = array.sort(function(a, b) {
@@ -19,6 +22,7 @@ function objectSort(array) {
   return sorted;
 };
 
+//nice!
 function distributeObjects(objectArray) {
   var sortedHighScoresEasy = [];
   var sortedHighScoresMed = [];
@@ -55,6 +59,10 @@ function populateScores(arrayOfScores, arrayOfScoreStrings) {
 //main
 
 highScores = getScores();
+//(continued from above) for instance:
+//sortedHighScores = highScores.sort(<sortfunction>)
+//or maybe even better, sort the array in the first line of the
+//function that puts them in various arrays
 sortedHighScores = objectSort(highScores);
 arrayOfScores = distributeObjects(sortedHighScores);
 populateScores(arrayOfScores, scoreStrings);
diff --git a/game.js b/game.js
index e5a7515..37a6dcb 100644
--- a/game.js
+++ b/game.js
@@ -1,5 +1,9 @@
 'use strict';
 
+//notice that all of these files start out with very similar paths.
+//you could save multiple lines by writing a function to concatenate these
+//paths rather than hard coding them.
+
 //arrays to hold data
 var animalsArray = ['imgs/animals/alligator.jpg', 'imgs/animals/bear.png', 'imgs/animals/cat.jpg', 'imgs/animals/chicken.jpg', 'imgs/animals/cow.jpg', 'imgs/animals/deer.jpg', 'imgs/animals/dog.jpg', 'imgs/animals/eagle.jpg', 'imgs/animals/elephant.jpg', 'imgs/animals/flamingo.jpg', 'imgs/animals/giraffe.jpg', 'imgs/animals/gorilla.jpg', 'imgs/animals/hippo.jpg', 'imgs/animals/horse.jpg', 'imgs/animals/lion.jpg', 'imgs/animals/lizard.jpg', 'imgs/animals/mouse.jpg', 'imgs/animals/parrot.jpg', 'imgs/animals/pig.jpg', 'imgs/animals/sheep.jpg', 'imgs/animals/sloth.jpg', 'imgs/animals/snake.jpg', 'imgs/animals/tiger.jpg', 'imgs/animals/turtle.jpg'];
 
@@ -47,6 +51,7 @@ function getGameParam(){
   key1 = level.cardset;
 };
 
+//nice!
 function shuffleArray(array){
   for (var i = array.length - 1; i > 0; i--){
     var j = Math.floor(Math.random() * (i + 1));
@@ -132,6 +137,9 @@ function updatePlayerInfo(countTotal) {
 };
 
 function buildTable(imageArray){
+  //what is iA!?  I know sometimes it get's to be a bit of a hurry when you're
+  //getting things to work but try to slow yourself down and name your
+  //variables a little more semantically
   var iA = 0;
   var table = document.getElementById('game_table');
   var tRow;
diff --git a/index.html b/index.html
index 44dc017..85397b5 100644
--- a/index.html
+++ b/index.html
@@ -7,6 +7,7 @@
   <link rel="stylesheet" href="style.css" type="text/css">
   <title></title>
 </head>
+  <!--A few too many divs. Keep in mind that mains are effectively divs in the first place. So if you just had an inner container (tablebackground) you could have just applied the styling to the main directly instead of adding an unnecessary container. So far as I can tell the divs: 'form_background', 'form_inputs', 'submit_button', 'nav_list' and could all be done away with. Similarly the classes 'navtop', 'navlink', 'form_label', and 'navlist' could have been removed in favor of using the proper css selectors. It's common early on to have a few too many divs and a few too many classes but try to keep that in mind especially early on when you structure your html.-->
 <body>
 <header>
   <h1>SCuTtle Your Memory!</h1>
@@ -20,6 +21,7 @@ <h1>SCuTtle Your Memory!</h1>
   </nav>
 </header>
 <main>
+
   <div class="tablebackground">
     <div class="instructions">
       <h2>Instructions</h2>
diff --git a/style.css b/style.css
index e4afe9f..e6b4b54 100644
--- a/style.css
+++ b/style.css
@@ -1,3 +1,11 @@
+/* There's a bit too much unnecessary relative positioning in this.
+You should pretty much never have to use relative positioning like that. With
+relative in particular it can cause weird problems because the element that you're
+positioning that way still takes up space at its original spot in the dom. If
+you have a situation where you need something on top of something else use absolute.
+If what you really need is just to move something around on the page, set the display
+such that you can manipulate it like you want and then use margin/padding.*/
+
 @font-face {
   font-family: 'EraserRegular';
   src: url('fonts/EraserRegular.ttf');
@@ -12,7 +20,7 @@
   font-family: 'DJB_Chalk_It_Up';
   src: url('fonts/DJB_Chalk_It_Up.ttf');
 }
-
+/* Try to avoid leaving commented out code in final submissions */
 /** {
   outline: 1px red dashed;
 }*/
@@ -93,9 +101,17 @@ nav.navfooter {
   width: 160px;
   height: 40px;
   margin: 0 10px;
+  /* As mentioned below the relative positioning could have been taken
+  out with something like this:
+  width: 148px;
+  height: 22px;
+  padding: 9px 6px;
+  */
 }
 
 a.navlink {
+  /* avoid relative positioning in situations like this
+  use padding on the container*/
   position: relative;
   font-size: 24px;
   font-family: 'Suez One', serif;
@@ -131,6 +147,7 @@ a:visited {
   font-size: 20px;
   color: black;
 }
+
 .hint img{
   position: relative;
   left: 48%;