Initial review comments, not intended for merge

pom.xml

@@ -11,6 +11,10 @@
     <properties>
         <maven.compiler.source>11</maven.compiler.source>
         <maven.compiler.target>11</maven.compiler.target>
+        <!-- You can also define dependency versions in the properties section for use below. -->
+        <!-- This is generally a best practice and helps when you have multiple dependencies -->
+        <!-- of the same library so you only have to change it in one place if it changes. -->
+        <junit.version>4.13.2</junit.version>
     </properties>
     <!--    maven allows us to use external dependencies from mvn repository.
             meaning, we're downloading java classes that other developers have written and can
@@ -20,7 +24,7 @@
             <!-- junit, our framework for writing unit tests.-->
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>
-            <version>4.13.2</version>
+            <version>${junit.version}</version>
         </dependency>
         <!-- https://mvnrepository.com/artifact/io.javalin/javalin -->
         <dependency>

src/main/java/Controller/Controller.java

@@ -9,6 +9,7 @@
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import io.javalin.Javalin;
+import io.javalin.http.HttpStatus;
 
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
@@ -17,12 +18,19 @@
 
 public class Controller {
 
-    VendorService vendorService;
-    ProductService productService;
+    // Constants are useful when reusing strings so you only have to modify in one play when changing.
+    // This can go in a dedicated constants file if desired.
+    public static final String VENDOR_ID_QUERY_PARAM = "vendor_id";
+    private VendorService vendorService;
+    private ProductService productService;
+    private ObjectMapper objectMapper;
 
-    public Controller(VendorService vendorService, ProductService productService){
+    public Controller(VendorService vendorService, ProductService productService) {
         this.vendorService = vendorService;
         this.productService = productService;
+        // Moving the initialization of object mapper to the constructor makes it get called once, instead of
+        // once per API request. More efficient code.
+        objectMapper = new ObjectMapper();
     }
 
 
@@ -32,14 +40,14 @@ public Javalin getAPI() {
         //VENDORS
         app.post("vendor", context -> {
             try {
-                ObjectMapper om = new ObjectMapper();
-                Vendor v = om.readValue(context.body(), Vendor.class);
-                vendorService.saveVendor(v);
-                context.status(201);
-                context.json(v);
+                Vendor vendor = objectMapper.readValue(context.body(), Vendor.class);
+                vendorService.saveVendor(vendor);
+                // Using constants can help with clarity
+                context.status(HttpStatus.CREATED);
+                context.json(vendor);
             } catch (JsonProcessingException e) {
                 context.result(e.getMessage());
-                context.status(400);
+                context.status(HttpStatus.BAD_REQUEST);
             }
         });
 
@@ -49,39 +57,43 @@ public Javalin getAPI() {
         });
 
         app.get("vendor/{vendor_id}", context -> {
-            int vendorID = Integer.parseInt(context.pathParam("vendor_id"));
-            try{
+            int vendorID = Integer.parseInt(context.pathParam(VENDOR_ID_QUERY_PARAM));
+            try {
                 Vendor v = vendorService.getVendorById(vendorID);
                 context.json(v);
-            }catch (VendorException e){
+            } catch (VendorException e) {
                 context.status(404);
                 context.result(e.getMessage());
             }
         });
 
         app.put("vendor/{vendor_id}", context -> {
-            ObjectMapper om = new ObjectMapper();
-            Vendor v = om.readValue(context.body(), Vendor.class);
-            int id = Integer.parseInt(context.pathParam("vendor_id"));
-            try{
+            Vendor v = objectMapper.readValue(context.body(), Vendor.class);
+            int id = Integer.parseInt(context.pathParam(VENDOR_ID_QUERY_PARAM));
+            try {
                 vendorService.updateVendor(v, id);
                 context.json(v);
-            }catch (VendorException e){
+            } catch (VendorException e) {
                 context.status(400);
                 context.result(e.getMessage());
             }
         });
 
 
-        //PRODUCTS
+        // Separating out logically separate code can help organize and enhance readability
+        initializeProductEndpoints(app);
+
+        return app;
+    }
+
+    private void initializeProductEndpoints(Javalin app) {
         app.post("product", context -> {
-            ObjectMapper om = new ObjectMapper();
-            Product p = om.readValue(context.body(), Product.class);
-            try{
+            Product p = objectMapper.readValue(context.body(), Product.class);
+            try {
                 productService.saveProduct(p);
                 context.status(201);
                 context.json(p);
-            }catch(ProductException e){
+            } catch (ProductException e) {
                 context.status(400);
                 context.result(e.getMessage());
             }
@@ -94,10 +106,10 @@ public Javalin getAPI() {
 
         app.get("product/{product_id}", context -> {
             int productID = Integer.parseInt(context.pathParam("product_id"));
-            try{
+            try {
                 Product p = productService.getProductById(productID);
                 context.json(p);
-            }catch (ProductException e){
+            } catch (ProductException e) {
                 context.status(400);
                 context.result(e.getMessage());
             }
@@ -106,23 +118,22 @@ public Javalin getAPI() {
         ///RETURNING ALL PRODUCTS
         app.get("product?soldBy={sold_by}", context -> {
             int soldBy = Integer.parseInt(context.pathParam("sold_by"));
-            try{
+            try {
                 List<Product> p = productService.getProductByVendor(soldBy);
                 context.json(p);
-            }catch (ProductException e){
+            } catch (ProductException e) {
                 context.status(400);
                 context.result(e.getMessage());
             }
         });
 
         app.put("product/{product_id}", context -> {
-            ObjectMapper om = new ObjectMapper();
-            Product p = om.readValue(context.body(), Product.class);
+            Product p = objectMapper.readValue(context.body(), Product.class);
             int id = Integer.parseInt(context.pathParam("product_id"));
-            try{
+            try {
                 productService.updateProduct(p, id);
                 context.json(p);
-            }catch (ProductException e){
+            } catch (ProductException e) {
                 context.status(400);
                 context.result(e.getMessage());
             }
@@ -133,8 +144,5 @@ public Javalin getAPI() {
             productService.deleteProduct(productID);
             context.status(200);
         });
-
-
-        return app;
     }
 }

src/main/java/DAO/VendorDAO.java

@@ -29,6 +29,26 @@ public void insertVendor(Vendor v){
         }
     }
 
+    //get vendor from table "Vendor" with a matching name
+    public List<Vendor> getVendorsByName(String vendorName) {
+        List<Vendor> vendorList = new ArrayList<>();
+        try{
+            PreparedStatement ps = conn.prepareStatement("select * from Vendor where vendor_name = ?");
+            ps.setString(1, vendorName);
+            ResultSet resultSet = ps.executeQuery();
+            while (resultSet.next()) {
+                int resultID = resultSet.getInt("vendor_id");
+                String resultName = resultSet.getString("vendor_name");
+                Vendor v = new Vendor(resultID, resultName);
+                vendorList.add(v);
+            }
+        }catch (SQLException e) {
+            //add better messaging
+            e.printStackTrace();
+        }
+        return vendorList;
+    }
+
     //get all vendors from table "Vendor"
     public List<Vendor> getVendors(){
         List<Vendor> vendorList = new ArrayList<>();

src/main/java/Model/Vendor.java

@@ -3,8 +3,10 @@
 import java.util.Objects;
 
 public class Vendor {
-    public int vendorId;
-    public String vendorName;
+    // We have getters and setter so that we can control how these fields are accessed. Making them
+    // private forces consumers to use these methods instead of accessing the fields directly.
+    private int vendorId;
+    private String vendorName;
 
     public Vendor(){
 

src/main/java/Service/VendorService.java

@@ -22,7 +22,9 @@ public VendorService() {
     public void saveVendor(Vendor v) throws VendorException {
         if (v.getVendorName().isEmpty()) {
             throw new VendorException("Vendor Name cannot be null");
-        } else if (vendorDAO.getVendors().contains(v)){  //update to be only name and not both params
+
+            // More efficient to not pull the whole database and instead pull a small subset as needed.
+        } else if (!vendorDAO.getVendorsByName(v.getVendorName()).isEmpty()){  //update to be only name and not both params
             throw new VendorException("Vendor already exists");
         }
         vendorDAO.insertVendor(v);

src/main/java/Util/ConnectionSingleton.java

@@ -19,7 +19,7 @@ public class ConnectionSingleton {
     /**
      * url will represent our connection string. Since this is an in-memory db, we will represent a file location to store the data
      */
-    private static String url = "jdbc:h2:./h2/db";
+    private static String URL = "jdbc:h2:./h2/db"; // SCREAMING_SNAKE_CASE is standard for constants.
     /**
      * Default username for connecting to h2
      */
@@ -40,7 +40,7 @@ public class ConnectionSingleton {
     public static Connection getConnection(){
         if(connection == null){
             try {
-                connection = DriverManager.getConnection(url, username, password);
+                connection = DriverManager.getConnection(URL, username, password);
                 resetTestDatabase();
             } catch (SQLException e) {
                 e.printStackTrace();