Completed HTML/CSS & JS 'WhatWouldYouChange' tests

dot-net/WhatWouldYouChange/INSTRUCTIONS.txt

@@ -5,4 +5,24 @@
 
 
 Suggested Changes:
-
+	`public string exampleText` maintains proper encapsulation using the `{ get; set; }` auto-properties, which is great! Originally, I was concerned that it was set to `public`, but I just learned about these auto-properties today.
+
+	In `ExampleMethod`, `var text = ""` should be declared with a type; in this instance, it should be `string text = ""` because we are always expecting it to be a string.
+
+	A StreamReader can be passed the txt file directly via the `exampleTextFile` parameter, so we don't need the `fs` variable.
+
+	The StreamReader should use a `using` statement so that even if an exception occurs, Dispose will still be called.
+
+	`catch` should really only be used for error handling. If we want to assign some default value if the operation fails, then that functionality should be captured in a condition check. There can be multiple reasons why an operation would fail and enter the `catch` block, and we might not necessarily always want the same thing to happen no matter how it fails.
+
+	As written, the construction of the "default" `text` variable inside of the `catch` block uses far more momory than it needs to since it is going through many reassignments. If the new lines are necessary, we can build up the `text` variable with the default "lorem ipsum" text using a StringBuilder and leverage `.AppendLine()` to be more efficient.
+
+	If the new lines are *not* necessary, then we can use simple string concatenation and break it into multiple lines in our editor instead of reassigning the `text` variable over and over. Example:
+		text = 
+			"Lorem ipsum dolor sit amet, consectetur adipiscing elit. " +
+			"Duis non pulvinar sapien, nec accumsan justo. " +
+			"etc."
+
+	When reassigning `exampleText`, we should use `this.exampleText` to properly reference it inside the class.
+
+	In `Program.cs`, `ExampleMethod` is being called with a string rather than a path to a valid .txt file. In this scenario, if we wanted to call the method with the `ExampleText.txt` file in the same directory, then it would need a valid file extension (ie. `ExampleTexttxt` !== `ExampleText.txt`).

html-css/WhatWouldYouChange/test.html

@@ -21,12 +21,42 @@
 Bonus point for the correct use of the word "semantic" in an answer.
 -->
 
-<div ID=redSection>
+<!-- Old Markup -->
+<!-- <div ID=redSection>
     <p class="recruitment-question">Select <b>your favorite</b> javascript framework:<br />
         <input type="checkbox" value="angular" /> Angular JS<br>
         <input type="checkbox" value="jquery" /> JQuery.<br>
         <input type="checkbox" value="reactjs" /> React JS<p>
-</div>
+</div> -->
+
+<!-- Every page should have an h1 for SEO purposes -->
+<h1>HTML &amp; CSS test</h1>
+<!-- id should be lowercase and the id name should be in quotes. However, a red section may be reusable, so I changed it to a class instead since id's should only be present once per page. The class name should also be separated by dashes, as is convention for css, and for consistency within this page -->
+<section class="red-section">
+    <!-- In HTML5, we can use the <section> tag to provide semantic meaning to our markup. This tells us that this form is a part of a group of content that has meaning no matter where on a page it is placed -->
+
+    <!-- If there are inputs we are trying to capture, this should be inside of a form tag, which would have some action or JS to capture submissions -->
+    <form>
+        <!-- <b> tags are outdated. <strong> is better, but setting font-weight: bold in CSS is best -->
+        <p class="recruitment-question">Select <strong>your favorite</strong> javascript framework:<br />
+
+        <!-- Sibling elements should be tabbed in at the same level for easier skimming of the markup -->
+        <!-- input type for should be "radio" when only one selection should be made, such as when you select a "favorite" item. When using type="radio", you must set the same "name" property so that only one choice can be provided -->
+        <input type="radio" name="framework" value="angular" id="angular" />
+        <label for="angular">Angular JS</label><br />
+        <!-- labels for inputs should be put in label tags with a for attribute pointing to the input's id for screen readers/accessibility -->
+        <!-- putting in the trailing "/" on a self closing tag is optional. They are present on the <input> tags, so I added them to the <br /> tags as well -->
+        <input type="radio" name="framework" value="jquery" id="jquery" />
+        <label for="jquery">jQuery</label><br />
+
+        <!-- previously, <br> tags were in use, and having an unclosed <p> tag is faulty markup -->
+        <input type="radio" name="framework" value="reactjs" id="reactjs" />
+        <label for="reactjs">React JS</label><br />
+
+        <!-- The form needs a submit button -->
+        <button type="submit">Submit</button>
+    </form>
+</section>
 
 </body>
 </html>

javascript/WhatWouldYouChange/test.html

@@ -1,47 +1,8 @@
 <!DOCTYPE html>
 <html lang="en">
 <head>
-<meta charset="utf-8">
-<title>JavaScript test: What would you change?</title>
-
-<script>
-  /*
-  Instructions
-  (~10 minutes):
-
-  This code appears to work. Nonetheless note up to 10 things that look bad to you, including a brief note as to why.
-
-  Please consider both minor syntactical issues with the code as it stands, and how the code is structured overall.
-   */
-
-  var submitButtonActive = false
-
-  function validateField() {
-    reqFlds = [];
-    reqFlds[0] = 'username';
-    reqFlds.push('password');
-
-    if(
-          document.getElementById(reqFlds[0]).value !== ''
-          &&
-          document.getElementById(reqFlds[1]).value !== ''
-        ) {
-          // valid
-          document.getElementById('submitButton').style.backgroundColor = '#00f';
-          return true;
-    }else {
-          document.getElementById('submitButton').style.backgroundColor = '#f00';
-          return
-            false;
-    }
-  }
-
-  document.getElementById('submitButton').onclick = function(){
-    submitButtonIsActive = validateField();
-    console.log('submitButtonActive', submitButtonActive)
-  }
-</script>
-
+  <meta charset="utf-8">
+  <title>JavaScript test: What would you change?</title>
 </head>
 <body>
 
@@ -50,5 +11,48 @@
 <p><label>Password <input type="password" id="password" name="password"/></label></p>
 <p><input id="submitButton" type="submit" value="submit" /></p>
 
+<script>
+    /*
+    Instructions
+    (~10 minutes):
+
+    This code appears to work. Nonetheless note up to 10 things that look bad to you, including a brief note as to why.
+
+    Please consider both minor syntactical issues with the code as it stands, and how the code is structured overall.
+     */
+
+    var submitButtonActive = false
+
+    function validateField() {
+      // store username and password in variables for use later. This will make our if condition more readable
+      const username = document.getElementById("username").value;
+      const password = document.getElementById("password").value;
+      // The commented code below was pushing a password and validating even if no password was entered. Since we're storing both username and password above from the input fields on the page, the reqFlds array is unnecessary
+      // reqFlds = [];
+      // reqFlds[0] = 'username';
+      // reqFlds.push('password');
+
+      // an empty string is falsy, so we just need to check if there is a value at all for username & password
+      if (username && password) {
+        document.getElementById('submitButton').style.backgroundColor = '#00f';
+        return true;
+      } else {
+        document.getElementById('submitButton').style.backgroundColor = '#f00';
+        // the bool that is returned should be on the same line for readability
+        return false;
+      }
+    }
+
+    document.getElementById('submitButton').onclick = function() {
+      submitButtonIsActive = validateField();
+      console.log('submitButtonActive', submitButtonActive)
+    }
+  </script>
+
+<!-- ideally, we would have our js in an external file and include it here -->
+<!-- <script src="example.js"></script> -->
+
+<!-- In lieu of an external JS file, I've moved the script tag containing our code to the end of the body so that the DOM loads before we start executing code that depends on it being present -->
+
 </body>
 </html>