zayanh · zayanh · Oct 30, 2016 · Oct 30, 2016 · Oct 30, 2016
diff --git a/exercise-003-web/server/users/home.html b/exercise-003-web/server/users/home.html
@@ -5,19 +5,14 @@
   <body>
     <h1> Welcome! </h1>
     <form action="/signup" method="POST">
-      <input type="text" name="user" placeholder="Enter desired username" />
-      <!-- <input type="text" name="user" value="kevin" /> -->
-      <input type="submit" value="Sign Up" />
+        <input type="text" name="user" placeholder="Enter desired username" />
+        <input type="submit" value="Sign Up" />
 
-
-      {{.Message}}
-
-      {{ range $key, $value := .UserNames }}
-        <li>{{ $key }}</li>
-      {{ end }}
-
-      <!-- <h1>I can see your name is {{ .Name }}</h1>
-      <h1>And you have been in existence for {{ .Age }} years</h1> -->
+        <ol>
+            {{ range $key, $value := .Usernames }}
+            <li>{{ $key }}</li>
+            {{ end }}
+        </ol>
     </form>
   </body>
 </html>
diff --git a/exercise-003-web/server/users/users.go b/exercise-003-web/server/users/users.go
@@ -9,109 +9,115 @@ import (
     "sync"
 )
 
-/* Keeping these variables global so Home and Signup can access them without them
-being redefined each time. */
-// data will hold the values to be passed into the HTML template
-type data struct {
-    /*UserNames is a map that holds the list of registered users.
-    Was initially thinking of making UserNames a slice of strings, but searching it for
-    duplicate entries would take O(n) time, while searching a map only takes O(1) time. */
-    UserNames map[string]bool
-    // Message will inform the user of the results from the previous attempt to add a new user
-    Message string
+
+// View will hold the values to be passed into the HTML template
+type view struct {
+    Usernames map[string]bool
 }
-var d data
+
+// All Usernames
+var usernames map[string]bool
+var usernamesMtx = &sync.Mutex{}
+
 // userFile is the file handle for the list of users
 var userFile *os.File // Is this okay?
+
 // homeT will store the html template to be used in this website
 var homeT *template.Template
 
-var mu = &sync.Mutex{}
-
 // Setup initializes the homeT variable which holds the HTML Template
 func Setup(dir string, filename string) {
+
     // homeT holds the HTML template to use when loading the website
     homeT = template.Must(template.ParseFiles(dir))
+
     // Initialize map of usernames
-    mu.Lock()
-    d.UserNames = make(map[string]bool)
-    mu.Unlock()
+    usernames = make(map[string]bool)
+
     // Open file to read and write. Create if it doesn't exist. Append if it does
     f, err := os.OpenFile(filename, os.O_CREATE | os.O_RDWR | os.O_APPEND, 0666)
     if err != nil {
-        //handle error
+        panic(err)
     }
-    userFile = f // use global variable -- is this approach okay?
 
-    // defer userFile.Close()  <-- hmmmmmmmmm
+    // use global variable
+    userFile = f
 
-    //Build the list
+    // Build the list
     loadUsers()
 }
 
 // addUserName is called when a new user is created from the website
 // first bool is if username is empty, second bool is if username exists
 func addUserName(username string) (bool, bool) {
+
     // Remove whitespace and check for an empty string
     if len(strings.TrimSpace(username)) <= 0 {
         return true, false
     }
 
     // Check if the username already exists
-    mu.Lock()
-    if _,ok := d.UserNames[username]; ok {
-        defer mu.Unlock() // is this okay?
+    usernamesMtx.Lock()
+    defer usernamesMtx.Unlock() // is this okay?
+    if _,ok := usernames[username]; ok {
         return false, true
     }
-    mu.Unlock()
 
     // If above checks pass, add username to database
-    mu.Lock()
-    d.UserNames[username] = true
-    mu.Unlock()
+    usernames[username] = true
     userFile.WriteString(username + "\n")
     return false, false
 }
 
 // loadUsers reads the provided file and builds the slice UserNames
 func loadUsers() {
-    // NewScanner returns a variable of type Scanner. A Scanner is a conveninent struct with
-    // useful methods for reading from an io.Reader object. It splits over \n by default
     scanner := bufio.NewScanner(userFile)
-
-    // Scan is a method for type Scanner. It generates the next "token" from scanner which can
-    // be read using the Text or Bytes methods. Scan returns false when it reaches the end of
-    // the input or encounters an error
     for scanner.Scan() {
-        mu.Lock()
-        d.UserNames[scanner.Text()] = true
-        mu.Unlock()
+        usernames[scanner.Text()] = true
     }
+
     // Err returns the first non-EOF error encoutnered by the scanner
     if err := scanner.Err(); err != nil {
-        // handle error
+        panic(err)
     }
 }
 
+func copyOfUsernames() map[string]bool {
+    result := make(map[string]bool)
+
+    usernamesMtx.Lock()
+    for name := range usernames {
+        result[name] = true
+    }
+    usernamesMtx.Unlock()
+
+    return result
+}
+
 // Signup is recalled every time the user clicks on the submit button
 func Signup(w http.ResponseWriter, r *http.Request) {
     r.ParseForm()
     username := r.Form.Get("user")
     empty, dup := addUserName(username)
+
+    // Leaving this logic in future implementation of printing a message
     switch {
     case dup:
-        d.Message = "Username already exists!"
     case empty:
-        d.Message = "Username can't be empty!"
     default:
-        d.Message = "User " + username + " added successfully"
     }
+
     http.Redirect(w, r, "/home", http.StatusFound)
 }
 
+
 // Home is where the website will redirect everytime it wants to display all the current usernames
 func Home(w http.ResponseWriter, r *http.Request) {
-    homeT.Execute(w, &d)
-    // Clear out the message incase Home is called again without a message being required
-    d.Message = ""
+
+    // Create a View
+    v := view{}
+    v.Usernames = copyOfUsernames()
+
+    // Execute the template
+    homeT.Execute(w, &v)
 }
diff --git a/exercise-003-web/server/users/users_test.go b/exercise-003-web/server/users/users_test.go
@@ -2,9 +2,6 @@ package users
 
 import (
 	"os"
-	// "fmt"
-	// "strings"
-	//"io"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -15,10 +12,12 @@ func init() {
 	// Create the file and append names to it
 	f, err := os.OpenFile("userList_test.dat", os.O_CREATE | os.O_RDWR | os.O_APPEND, 0666)
 	if err != nil {
-		// do something
+		panic(err)
 	}
 	f.WriteString("Kevin\n")
 	f.WriteString("Lorelei\n")
+
+	// Run setup with provided html and file just created
 	Setup("home.html", "userList_test.dat")
 }
 
@@ -28,30 +27,37 @@ func TestServer(t *testing.T) {
 	assert.Nil(err)
 	w := httptest.NewRecorder()
 
+	// Make sure the users were loaded correctly
 	Home(w, req)
 	assert.Contains(w.Body.String(), "Welcome")
 	assert.Contains(w.Body.String(), "Kevin")
 	assert.Contains(w.Body.String(), "Lorelei")
 
+	// Add a new user
 	req.ParseForm()
 	req.Form.Set("user", "George")
 	Signup(w, req)
 	Home(w, req)
-	assert.Contains(w.Body.String(), "User George added successfully")
+	assert.Contains(w.Body.String(), "George")
+
+	// Make sure there are only 3 users
+	assert.NotContains(w.Body.String(), "4.")
 
+	// Duplicate username should not be added
 	req.Form.Set("user", "George")
 	Signup(w, req)
 	Home(w, req)
-	assert.Contains(w.Body.String(), "Username already exists")
+	assert.NotContains(w.Body.String(), "4.")
 
-	// Can't handle the string "can't"
+	// Empty username should not be added
 	req.Form.Set("user", "")
 	Signup(w, req)
 	Home(w, req)
-	assert.Contains(w.Body.String(), "be empty")
+	assert.NotContains(w.Body.String(), "4.")
 
 	// Check if the redirect is working??
 	assert.True(assert.HTTPRedirect(Signup, "GET", "localhost:8080/home", nil))
 
+	// Cleanup
 	os.Remove("userList_test.dat")
 }