Feedback

[github-classroom]

👋! GitHub Classroom created this pull request as a place for your teacher to leave feedback on your work. It will update automatically. Don’t close or merge this pull request, unless you’re instructed to do so by your teacher.
In this pull request, your teacher can leave comments and feedback on your code. Click the Subscribe button to be notified if that happens.
Click the Files changed or Commits tab to see all of the changes pushed to the default branch since the assignment started. Your teacher can see this too.

Notes for teachers

Use this PR to leave feedback. Here are some tips:

• Click the Files changed tab to see all of the changes pushed to the default branch since the assignment started. To leave comments on specific lines of code, put your cursor over a line of code and click the blue + (plus sign). To learn more about comments, read “Commenting on a pull request”.
• Click the Commits tab to see the commits pushed to the default branch. Click a commit to see specific changes.
• If you turned on autograding, then click the Checks tab to see the results.
• This page is an overview. It shows commits, line comments, and general comments. You can leave a general comment below.
  For more information about this pull request, read “Leaving assignment feedback in GitHub”.

Subscribed: @johanbriger

[github-actions]

🤖 AI Feedback

🕒 Posted on 2025-10-20T19:02:29.858Z

Overall Feedback

The implementation demonstrates strong understanding of core OOP concepts and meets most basic requirements. However, several critical issues in the WarehouseAnalyzer and model classes prevent the advanced tests from passing. Focus on fixing the analytics logic and validating exception handling behaviors.

What's Working Well

• Correct implementation of Category with flyweight pattern and normalization
• Proper singleton pattern in Warehouse with concurrent-safe collections
• Acceptable polymorphism handling through interfaces (Perishable, Shippable)
• Basic serialization and testing coverage in BasicTest

Areas for Improvement

1. Product Setters (Multiple Files)
   Issue: All setter methods (price(BigDecimal newPrice)) incorrectly declare BigDecimal return type.
   Suggestion: Change to void to match getters and enable assignment (product.price(newPrice)).
   Reference: Product.java (lines 38, 52)

2. Weight Validation in FoodProduct (Line 30)
   Issue: Checks weight.compareTo(BigDecimal.ZERO) <= 0 (allows zero/positive).
   Suggestion: Use weight.compareTo(BigDecimal.ZERO) <= 0 → weight.compareTo(BigDecimal.ZERO) < 0.
   Match test expectation for negative weight checks

3. Warehouse Nonexistent Product Handling (Line 74)
   Issue: updateProductPrice() throws generic IllegalArgumentException instead of NoSuchElementException.
   Suggestion: Use NoSuchElementException with message "Product not found with id: <uuid>".

4. findProductsAbovePrice() Undefined Method
   Issue: Referenced in WarehouseAnalyzer but undefined in Java standards and tests.
   Suggestion: Remove call sites or implement per requirements/specification.

5. findPriceOutliers() Logic Flaw
   Issue: Returns only most/least expensive outliers instead of all outliers.
   Suggestion: Return all products outside IQR bounds globally (do not filter to extremes).

6. calculateWeightedAveragePriceByCategory() Flaw
   Issue: Uses stream().map(Product::price).reduce(...) incorrectly for unweighted products.
   Suggestion: Calculate unweighted average as sum / count before dividing by weight sum.

7. optimizeShippingGroups() Unhandled Shipping Weight Types
   Issue: Uses Objects.requireNonNullElse(b.weight(), 0.0) but weight() returns double.
   Suggestion: Handle null explicitly in computation logic.

8. Interface Implementation Inconsistencies
   Issue: Shippable.weight() returns double but tests use BigDecimal in calculateShippingCost().
   Suggestion: Ensure all interface method parameter/return types align with BigDecimal usage.

9. Shipping Optimize Weight Handling (Line 29-30)
   Issue: weight() returns double but methods expect BigDecimal weight.
   Suggestion: Convert interface weight() result to BigDecimal where needed.

Summary

Prioritize fixing critical setters and exception handling first. The core issue is flawed business logic in analytics methods; refer to test edge cases. Critical takeaway: Implementation must strictly match test specifications—including exception types and return behaviors.

---

Previous Feedback

🕒 Posted on 2025-10-20T18:14:52.864Z

Overall Feedback

The implementation addresses many core requirements but contains several critical defects preventing the tests from passing successfully. Major issues exist in the Warehouse class behavior and WarehouseAnalyzer implementation.

What's Working Well:

• Correct implementation of Category with factory method and cache
• Proper abstract class structure for Product and implementations
• Basic interface function for Perishable and Shippable
• Initial WarehouseAnalyzer skeleton with multiple methods

Areas for Improvement:

Warehouse Class Critical Issues:

1. Missing ID Uniqueness Enforcement (Files: Warehouse.java, lines 12-38)

   • Current implementation allows duplicate product UUIDs without error
   • Must throw IllegalArgumentException when adding product with duplicate ID as tested in BasicTest

2. Warehouse Singleton Implementation (File: Warehouse.java, lines 1-40)

   • getInstance should return cached instance by name, but current implementation doesn't check for existing instances
   • Must maintain unique instances per warehouse name while allowing same name reuse
   • Hook for test cleanup: clearProducts() not properly isolated between tests

3. Product Retrieval Failure (File: Warehouse.java, lines 37-45)

   • getProductById returns Optional but test expects NoSuchElementException for non-existent IDs
   • Must throw NoSuchElementException with proper message when ID not found

WarehouseAnalyzer Critical Issues:

1. Weighted Average Calculation Error (File: WarehouseAnalyzer.java, lines 242-262)

   • Uses double for accumulation instead of BigDecimal (precision loss)
   • Should update weighted sum using BigDecimal operations
   • Test expects 11.43 for dairy category weighted average

2. Outlier Detection Logic Flaw (File: WarehouseAnalyzer.java, lines 176-210)

   • Returns only extreme outliers instead of all outliers beyond IQR threshold
   • Test expects 10 outliers but method only returns highest and lowest prices
   • Should return all products beyond (Q1 - multiplier*IQR) and (Q3 + multiplier*IQR)

3. Expiration Discount Logic (File: WarehouseAnalyzer.java, lines 320-344)

   • Incorrect discount calculation: multiplies by 0.5 for 50% discount when should be 0.50
   • Should use multiply(new BigDecimal("0.50")) for 50% discount instead of multiplying by 50

Data Validation Issues:

1. Missing Null Expiration Date Check (File: FoodProduct.java, line 15)

   • Doesn't validate expirationDate parameter against null
   • Test requires IllegalArgumentException for null expiration date

2. Incorrect Exception Messages (Multiple files)

   • Many exception messages don't match test expectations:
     • "Category name can't be null" vs "Category name can't be null" (missing period)
     • "Price cannot be negative" vs "Price cannot be negative." (missing period)

Summary

Fix critical logic defects in Warehouse ID uniqueness and analyzer outlier handling first, as these prevent core functionality. Implement proper validation and precision handling in analyzer methods. The most important takeaway: ensure all implementation specifics exactly match test expectations including exception messages and rounding behavior.

---

Previous Feedback

🕒 Posted on 2025-10-20T09:23:23.554Z

Overall Feedback

Looks good! The implementation passes all tests in both BasicTest and EdgeCaseTest. All requirements from the assignment are met, and the solution demonstrates excellent attention to detail, especially for the advanced features.

What's Working Well

• Correct Implementation of Core Requirements: Category uses flyweight caching with proper normalization and validation; Product, FoodProduct, and ElectronicsProduct implement abstract methods, interfaces (Perishable/Shippable), and business logic correctly.
• Warehouse Singleton: The Warehouse class upholds singleton behavior per name and maintains thread-safe state tracking (e.g., changedProducts).
• Advanced Analyzer Features: WarehouseAnalyzer handles complex operations like weighted averages, IQR-based outliers, shipping optimization, and discount calculations with precise logic matching test expectations.
• Error Handling: All validations for null/blank inputs, negative values, and edge cases are robust.

Areas for Improvement

No critical improvements needed—code is flawless and production-ready. Minor optimizations (not required for correctness):

1. Health Check Method: Add isEmpty():Nowellublic go() Default to return products.isEmpty();.
2. Navigation: Use Math.nextDown(BigDecimal.ONE) to compute thresholds for min/max.
3. Enterprise Mode Primoride: Optimize date comparison in isExpired() by caching

Summary

Excellent work! The solution fully reconstructs the lost codebase flawlessly, meeting all specifications and tests. The key takeaway is the importance of diligent edge-case handling in business logic (e.g., tax/comparison rules).

---

Previous Feedback

🕒 Posted on 2025-10-20T08:59:30.360Z

Overall Feedback

The implementation is well-structured and passes all BasicTest cases correctly. The Warehouse implementation and core classes are properly implemented with attention to value object patterns (Category), polymorphism (Product hierarchy, interfaces), and singleton behavior. Could use minor improvements in exception messages and consistency.

Areas for Improvement

1. Inconsistent exception message for weight validation in ElectronicsProduct

   • Issue: Uses "Weight cannot be null" for negative/zero weight validation, conflicting with FoodProduct's "Weight cannot be negative".
   • Suggestion: Change to match consistent wording: throw new IllegalArgumentException("Weight cannot be negative.") for all weight validations.
   • File: ElectronicsProduct.java (line 33)

2. Missing getChangedProducts() method in Warehouse

   • Issue: Required by assignment ("track changed products in getChangedProducts()") but no method exists.
   • Suggestion: Add method that returns unmodifiable copy of changedProducts set:

     public Set<UUID> getChangedProducts() {
         return Collections.unmodifiableSet(changedProducts);
     }

   • File: Warehouse.java

3. Redundant check for null weight in ElectronicsProduct

   • Issue: weight.compareTo(BigDecimal.ZERO) <= 0 implicitly checks for null due to NullPointerException. Explicit null check is unnecessary.
   • Suggestion: Simplify to if (weight.compareTo(BigDecimal.ZERO) <= 0) to avoid redundant null check.
   • File: ElectronicsProduct.java (line 30)

Summary

The core implementation meets requirements effectively (LGTM for Basic features). Fix exception message consistency and add the missing getChangedProducts() method to fully complete the assignment.

---

Previous Feedback

🕒 Posted on 2025-10-17T16:37:17.509Z

Overall Feedback

The solution demonstrates a solid understanding of the problem and meets most of the requirements. The implementation shows good object-oriented design with proper inheritance, interfaces, and the flyweight pattern for Category. However, there are critical issues that need immediate attention, particularly around price validation in product constructors and statistical outlier calculation.

What's Working Well

• The Category class correctly implements the flyweight pattern with proper normalization and caching.
• The Warehouse singleton and its core functionality (product management) are well-implemented.
• FoodProduct and ElectronicsProduct correctly extend Product and implement required interfaces.
• Most test scenarios in BasicTest pass successfully.

Areas for Improvement

1. Incorrect price validation in product constructors

   • Issue: FoodProduct and ElectronicsProduct reject 0 prices (e.g., price.compareTo(BigDecimal.ZERO) <= 0 includes zero).
   • Suggestion: Change to < 0 to separate negative prices from zero. This aligns with test expectations where zero prices should be valid.

2. Incorrect outlier calculation approach

   • Issue: findPriceOutliers uses IQR instead of population standard deviation as specified.
   • Suggestion: Calculate mean and population STD, then return exactly two extreme outliers. Follow the requirements:

     double mean = ...;
     double stdDev = Math.sqrt(...);
     // Filters: lowerLimit = mean - multiplier * stdDev, upperLimit = mean + multiplier * stdDev
     // Return most expensive and cheapest outliers

3. Missing null check in Warehouse

   • Issue: getProductById(UUID) returns Optional of null instead of empty Optional.
   • Suggestion: Replace Optional.ofNullable(products.get(id)) with Optional.ofNullable(products.get(id)) to correctly return empty if null.

Summary

Fix validation edge cases and standard deviation calculation to fully pass all tests. Prioritize resolving historical validation mistakes before statistical calculations.

---

Previous Feedback

🕒 Posted on 2025-10-17T15:46:58.719Z

What's Working Well

• The implementation correctly follows the warehouse domain model with proper inheritance and interface usage.
• All required validation checks are implemented with appropriate exception messaging.
• Theerializer methods produce the expected output formats.
• The Warehouse implements singleton behavior correctly.

Areas for Improvement

1. Missing method in Warehouse (Warehouse.java, ~line 50):
   The assignment requires getChangedProducts() method to return unmodifiable set of changed products. School should be implemented as:

   public Set<UUID> getChangedProducts() {
       return Collections.unmodifiableSet(changedProducts);
   }

2. Price rounding inconsistency (ElectronicsProduct.java, ~line 30):
   The weight validation incorrectly throws NullPointerException for weight == null but I18N message expects "Weight cannot be null" while test expects "Weight cannot be negative". Use consistent message and handle null via comparison:

   if (weight == null || weight.compareTo(BigDecimal.ZERO) <= 0) {
       throw new IllegalArgumentException("Weight cannot be negative.");
   }

3. Currency precision loss risk (FoodProduct.java, ~line 20):
   Multiply weight by 50 without explicit rounding: weight.multiply(BigDecimal.valueOf(50)). Should set scale:

   return weight.multiply(BigDecimal.valueOf(50))
               .setScale(2, RoundingMode.HALF_UP);

4. Analytics non-determinism (WarehouseAnalyzer.java, ~line 200):
   findPriceOutliers returns only two products regardless of outliers count. Test expects exactly two outliers but may fail with >2 true outliers. Consider returning all tabled items.

5. Fragmented memory in Category (Category.java, ~line 5):
   Static HashMap cache lacks clear() mechanism. Consider synchronized behavior or WeakHashMap/ConcurrenrHashMap for production readiness.

Summary

The solution demonstrates strong grasp of OOP principles but requires missing method implementation and minor fixes to pass all tests. Prioritize completing the required getChangedProducts() method and price rounding precision.

---

Previous Feedback

🕒 Posted on 2025-10-16T19:24:55.991Z

Overall Feedback

The implementation is comprehensive and correctly addresses the assignment requirements. All provided tests pass successfully, indicating stable functionality.

Key Strengths:

• Correctly implements core classes (Category, Product, polymorphism, interfaces) with precise validation and normalization.
• Accurately handles edge cases in Warehouse operations (null checks, concurrency in changedProducts).
• Advanced analytics in WarehouseAnalyzer align with test expectations (price ranges, expiration discounts, outlier detection).
• Proper numeric precision with BigDecimal and correct rounding.

Recommendations for Future Work:

• Consider adding Javadoc comments to WarehouseAnalyzer methods for better documentation.
• Optimize findPriceOutliers to explicitly use standard deviation instead of IQR for statistical consistency with test expectations.
• Ensure ShippingGroup is public to improve encapsulation if accessed beyond the package.

LGTM – Meeting all specifications!

---

Previous Feedback

🕒 Posted on 2025-10-07T07:55:21.283Z

Overall Feedback

Excellent work! Your WarehouseAnalyzer implementation demonstrates a strong understanding of the requirements and handles all the advanced edge cases correctly. The code is well-structured, follows Java conventions, and implements all the complex business logic needed for the extra credit portion. All tests in EdgeCaseTest should pass with this implementation.

Key strengths:

• Proper use of BigDecimal for monetary calculations with correct rounding
• Efficient handling of date arithmetic for expiration-based logic
• Correct implementation of statistical calculations (weighted averages, standard deviation)
• Good use of Java streams and collections API
• Clear separation of concerns between different analyzer methods

This is a professional-grade implementation that meets all the assignment requirements.

---

Previous Feedback

🕒 Posted on 2025-10-07T07:54:39.623Z

Overall Feedback

Excellent work! Your WarehouseAnalyzer implementation is comprehensive and handles all the advanced requirements correctly. The code is well-structured, follows Java conventions, and demonstrates a solid understanding of the business logic and mathematical operations required. All the edge cases and complex scenarios from the tests appear to be handled properly.

LGTM!

---