From b6bee88fdb0cb8e6df1b714f95996644cb6ac7a7 Mon Sep 17 00:00:00 2001
From: Zachary Klein <zak@silver-chalice.com>
Date: Fri, 27 Jul 2018 18:53:14 -0500
Subject: [PATCH] Added comments

---
 .../notepad/NotepadController.groovy          |  3 +++
 .../notepad/NotepadControllerTest.groovy      | 22 +++++++++++++++----
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/grails-app/controllers/notepad/NotepadController.groovy b/grails-app/controllers/notepad/NotepadController.groovy
index a6eb640..2346bc2 100644
--- a/grails-app/controllers/notepad/NotepadController.groovy
+++ b/grails-app/controllers/notepad/NotepadController.groovy
@@ -16,6 +16,7 @@ class NotepadController {
     def saveMethod() {
 
         def saveNotes = new Notepad(params)
+        //TODO: System.out.* is imported automatically in Groovy - you can just use `println` - Zak
         System.out.println("the value of params in controller = " + params) //scaffold
         saveNotes.save()
         redirect(action: 'current')
@@ -25,7 +26,9 @@ class NotepadController {
 
         Notepad notepadInstance = Notepad.get(id)
 
+        //TODO: Can be shortened to if(!notepadInstance) - Google "Groovy truth" for details - Zak
         if (notepadInstance == null) {
+            //TODO: System.out. is unnecessary
             System.out.println("Docuemnt Object is NULL")
             return
         }
diff --git a/src/test/groovy/notepad/NotepadControllerTest.groovy b/src/test/groovy/notepad/NotepadControllerTest.groovy
index 3f938d8..840f01c 100644
--- a/src/test/groovy/notepad/NotepadControllerTest.groovy
+++ b/src/test/groovy/notepad/NotepadControllerTest.groovy
@@ -16,15 +16,29 @@ import java.sql.Statement;
 @Mock(Notepad)
 class NotepadControllerTest extends Specification {
 
+    //TODO: Unit tests should not be used to test against databases.
+    // GORM (the persistence framework used by Grails) is not running
+    // during unit tests. The proper place to test such functionality
+    // is within an *integration test*. https://docs.grails.org/latest/guide/testing.html#integrationTesting
+    //
+    // Also, even within an integration test, opening a direct JDBC
+    // connection is brittle and not usually necessary - you can simply
+    // use GORM to check for whether the record was persisted by calling
+    // Notepad.get() (or Notepad.list() to retrieve all of the records) - Zak
+
+    //A better use of unit tests for your controller might be (for example) to check
+    // if a redirect is issued when you expect (like your redirect to the 'current' action)
+
     @Shared
-    Connection conn
+    Connection conn //TODO: As described above, you shouldn't use direct SQL connections
+                    // in a unit test (or really any Grails test, usually)
 
 
     @Shared
-    Statement stmt
+    Statement stmt //TODO: Same as above
 
     @Shared
-    ResultSet rs
+    ResultSet rs //TODO: Same as above
 
     /* Initialization logic here - connection over jdbc to h2 database*/
     def setupSpec() {
@@ -79,7 +93,7 @@ class NotepadControllerTest extends Specification {
 
         when:
         controller.saveMethod()
-        viewDb()
+        viewDb() //TODO: As described above, persistence to the database will not work in a unit test
 
         then:
         response.redirectedUrl == "/notepad/current"