From 0a69fe6555b88a777bd1437ee444635f208a9d4b Mon Sep 17 00:00:00 2001
From: Nathan Woodburn <github@nathan.woodburn.au>
Date: Mon, 1 May 2023 17:49:54 +1000
Subject: [PATCH] admin: Added contribution, originality and code review

---
 admin/E-contribution.yml   |  8 ++++----
 admin/E-originality.yml    | 26 ++++++++++++-------------
 admin/E-review-u7156831.md | 40 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 17 deletions(-)
 create mode 100644 admin/E-review-u7156831.md

diff --git a/admin/E-contribution.yml b/admin/E-contribution.yml
index 5183a6f..65a5856 100644
--- a/admin/E-contribution.yml
+++ b/admin/E-contribution.yml
@@ -8,8 +8,8 @@ declaration: >-
 # be 100 or 99 (33/33/33 is fine).  (Remove entries if you have fewer than three
 # members).  
 contributions:
-  - uid: 
-    contribution: 
+  - uid: u7156831
+    contribution: 33 
   - uid:
     contribution:
   - uid: u7280427
@@ -18,8 +18,8 @@ contributions:
 # Sign *your* name and uids here. (Remove entries if you have fewer
 # than three members)
 signatures:
-  - name:
-    uid:
+  - name: Nathan Woodburn
+    uid: u7156831
   - name:
     uid:
   - name: Immanuel Alvaro Bhirawa
diff --git a/admin/E-originality.yml b/admin/E-originality.yml
index 3a4ece6..5d21532 100644
--- a/admin/E-originality.yml
+++ b/admin/E-originality.yml
@@ -19,9 +19,9 @@ declaration: >-
 #
 # Add as many "name+comment" entries as necessary
 # (or remove it altogether if you haven't collaborated with anyone)
-collaboration:
-  - name:
-    comment: >-
+# collaboration:
+#   - name:
+#     comment: >-
 
 # Use this to list any code that you used that you did not write,
 # aside from code provided by the lecturer.  Provide a comment
@@ -30,10 +30,10 @@ collaboration:
 #
 # Add as many "url+licence+comment" entries as necessary
 # (or remove it altogether if you haven't used any external code)
-code:
-  - comment:
-    url:
-    licence:
+# code:
+#   - comment:
+#     url:
+#     licence:
 
 # Use this to list any assets (artwork, sound, etc) that you used.
 # Provide a comment explaining your use of that asset and the URL
@@ -41,17 +41,17 @@ code:
 #
 # Add as many "url+licence+comment" entries as necessary
 # (or remove it altogether if you haven't used any external assets)
-assets:
-  - comment:
-    url:
-    licence:
+# assets:
+#   - comment:
+#     url:
+#     licence:
 
 
 # Sign *your* name and uids here. (Remove entries if you have fewer
 # than three members.)
 signatures:
-  - name:
-    uid:
+  - name: Nathan Woodburn
+    uid: u7156831
   - name:
     uid:
   - name:
diff --git a/admin/E-review-u7156831.md b/admin/E-review-u7156831.md
new file mode 100644
index 0000000..e829de1
--- /dev/null
+++ b/admin/E-review-u7156831.md
@@ -0,0 +1,40 @@
+## Code Review
+
+Reviewed by: Nathan Woodburn, u7156831
+
+Reviewing code written by: Immanuel Alvaro Bhirawa, u7280427
+
+Component: `isMoveValid` from [BlueLagoon.java L145-L311](https://gitlab.cecs.anu.edu.au/u7156831/comp1110-ass2/-/blob/master/src/comp1110/ass2/BlueLagoon.java#L145-L311)
+
+### Comments
+
+- Good practices used:  
+  - Using other functions to help with the main function, such as `isStateStringWellFormed` and `isMoveStringWellFormed`.
+  - Using good variable names such as `currentPhase` and `boardHeight`
+  - Using a `switch case` statement instead of multiple `if` statements.
+  - Using `.addAll()` to add multiple elements to an arraylist instead of iterating through each element and adding them
+  - Good use of `for` loops to iterate through arrays
+
+- Places to improve:  
+  - Some comments are not needed, such as commenting about well named variables.
+  ```java
+  int numberOfPlayer = 0; // Number of player
+  String playerId = ""; // Player ID
+  ```
+  - Duplicate code could be avoided. For example [this switch](https://gitlab.cecs.anu.edu.au/u7156831/comp1110-ass2/-/blob/b8487c3c0826bef4e676a13f8ea05c578c73d2de/src/comp1110/ass2/BlueLagoon.java#L231-L256) could be shortened to the below to avoid duplicate code.
+    ```java
+    switch (numberOfPlayer) {
+        case 4 -> numberOfSettlersPerPlayer = 20;
+        case 3 -> numberOfSettlersPerPlayer = 25;
+        case 2 -> numberOfSettlersPerPlayer = 30;
+    }
+    if (pieceType.equals("S")) {
+        if (settlerCounter + 1 > numberOfSettlersPerPlayer) return false;
+    } else if (pieceType.equals("T")) {
+        if (villageCounter + 1 > numberOfVillagesPerPlayer) return false;
+    }
+    ```
+    - A few parts of code could be moved outside of the loop to improve efficiency and speed. For example the above snippet could be moved to the part of the code where numberOfPlayer is set. 
+
+- Possible errors
+  - `numberOfSettlersPerPlayer -= 10;` (and the `-5` version) could create the incorrect number as this is inside a loop. This could be avoided by setting the variable to a constant value instead of subtracting 10 each time.
\ No newline at end of file