Skip to content

Conversation

@github-classroom
Copy link

@github-classroom github-classroom bot commented Oct 12, 2025

👋! 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: @Cavve

@github-actions
Copy link

github-actions bot commented Oct 12, 2025

🤖 AI Feedback

🕒 Posted on 2025-10-25T16:04:44.514Z

Overall Feedback

The submission passes most tests but has critical calculation errors in weighted averages and shipping costs. Several methods contain logic flaws that cause test failures.

Areas for Improvement

  1. Weighted Average Calculation

    • Issue: SHIPWABLE check uses weight()() instead of weight()
    • Suggestion: Correct method call to s.weight() to retrieve weight directly
    if (p instanceof Shippable s) {  
      double w = s.weight();  // Fixed: direct invocation  
      ...  
    }  

    Impact: Causes incorrect weighted average results

  2. Shipping Cost Calculation

    • Issue: calculateShippingCost() uses BigDecimal-to-double multiplication
    • Suggestion: Maintain full precision with multiply() instead of doubleValue()
    // FoodProduct.calculateShippingCost()  
    // Incorrect: return BigDecimal.valueOf(weight.doubleValue() * 50);  
    return weight.multiply(BigDecimal.valueOf(50));  // Fixed: precise calculation  

    Impact: Rounded shipping costs cause test failures

  3. Expiration Check Logic

    • Issue: findProductsExpiringWithinDays() includes items expiring after end date
    • Suggestion: Fix boundary check to exclude items after end date
    if (!exp.isBefore(today) && exp.isBefore(end.plusDays(1))) {  // Fixed: exclusive upper bound  
      result.add(per);  
    }  

    Impact: Incorrect products included in expiration window

  4. Unsynchronized Set

    • Issue: changedProducts in Warehouse is unsynchronized
    • Suggestion: Initialize with Collections.synchronizedSet() for thread safety
    private final Set<Product> changedProducts = Collections.synchronizedSet(new HashSet<>());  

    Impact: Potential concurrency issues in multi-threaded environments

Summary

Priority is correcting calculation precision and boundary logic. The most critical flaw is the weighted average calculation method call error. Address these first for full test success.


Previous Feedback

🕒 Posted on 2025-10-24T13:59:52.506Z

What's Working Well

  • The implementation of core classes (Category, Product, FoodProduct, ElectronicsProduct, Warehouse) correctly passes all basic tests.
  • The Warehouse class properly handles singleton behavior, product management, and polymorphic interactions.
  • All exception handling for input validation (nulls, negative values) is correctly implemented with exact exception messages.

Areas for Improvement

  1. Weighted Average Calculation Defect
    Issue: The calculateWeightedAveragePriceByCategory() method incorrectly calculates averages when no items have positive weight.
    Suggestion:

    // Replace the entire else block with:
    else {
        BigDecimal sum = items.stream().map(Product::price).reduce(BigDecimal.ZERO, BigDecimal::add);
        avg = sum.divide(new BigDecimal(items.size()), 2, RoundingMode.HALF_UP);
    }

    Explanation: The current implementation divides weighted sum (always zero) by item count, returning incorrect averages. The fix uses the arithmetic mean of all products when no shippable items have positive weight.

  2. Limited Handling in Price Outlier Calculation
    Issue: The findPriceOutliers method uses a fixed IQR multiplier (1.5) instead of accommodating parameter input.
    Suggestion:

    // Change method parameter to accept IQR multiplier
    public List<Product> findPriceOutliers(double iqrMulti) { ... }

    And adjust calculations:

    double lowerBound = Q1 - (iqrMulti * IQR); // Using method parameter
    double upperBound = Q3 + (iqrMulti * IQR);

    Explanation: The test passes with iqrMulti = 2.0, but using a fixed value instead of the parameter iqrMulti violates the contract.

  3. List Mutation in Warehouse
    Issue: Warehouse.getProducts() returns an unmodifiable list but creates a new list internally, potentially duplicating large datasets.
    Suggestion: Return the original list directly:

    // Modify getProducts() to:
    public List<Product> getProducts() {
        return Collections.unmodifiableList(products); // Direct unmodification instead of copying
    }

    Explanation: Using List.copyOf() performs unnecessary list duplication while Collections.unmodifiableList() provides better performance.

  4. Type Safety in Shipping Group Calculation
    Issue: The optimizeShippingGroups method uses raw types (List, Map) leading to unchecked warnings.
    Suggestion: Declare types explicitly:

    // Use parameterized types instead of raw types
    private Map<Category, List<Product>> getProductsGroupedByCategories() { ... }
    private List<Shippable> shippableProducts() { ... }

    Explanation: Using raw types reduces type safety and produces compiler warnings for proper generics.

Summary

Fix the weighted average calculation for categories without shippable items, parameterize the IQR multiplier in outlier detection, optimize list immutability in the warehouse, and eliminate raw types for better type safety. The most critical improvement is correcting the weighted average calculation, which will prevent incorrect inventory analytics.


Previous Feedback

🕒 Posted on 2025-10-22T09:43:25.658Z

Overall Feedback

The submission demonstrates strong Java skills with robust implementations across all required classes and analyzer methods. However, several critical issues need addressing before all tests pass.

What's Working Well

  • Effective use of Java 9+ features like computeIfAbsent for flyweight patterns in Category and Warehouse
  • Proper implementation of abstract polymorphism in Product hierarchy
  • Functional acceptance of all basic tests through careful interface implementation
  • Comprehensive progress on advanced features with accurate weighted averaging logic

Areas for Improvement

  1. Warehouse.getProducts() returns mutable list

    • Issue: Returns ArrayList after stream.toList() instead of optimized unmodifiable list
    • Suggestion: Change to List.copyOf(products) to enforce immutability as required by test expectations
  2. Changed products tracking in Warehouse.updateProductPrice()

    • Issue: Possible duplicate entries in changedProducts when multiple updates occur
    • Suggestion: Use Collections.synchronizedList or validate before adding to ensure uniqueness
  3. Missing negative weight validation in product classes

    • Issue: Both FoodProduct and ElectronicsProduct constructors accept negative weights
    • Suggestion: Add null/zero checks in constructors:
      if (weight.compareTo(BigDecimal.ZERO) <= 0) {
          throw new IllegalArgumentException("Weight cannot be negative or zero");
      }
  4. Price outlier algorithm edge case handling

    • Issue: Incorrect Q1/Q3 calculation when dataset size isn't divisible by 4
    • Suggestion: Use proper median calculation:
      double median(List<Product> list) {
          int size = list.size();
          return (size % 2 == 0) 
                  ? (list.get(size/2 - 1).price().doubleValue() + list.get(size/2).price().doubleValue()) / 2 
                  : list.get(size/2).price().doubleValue();
      }
  5. InventoryValidation threshold mismatch

    • Issue: Hardcoded 1000 threshold doesn't match test data (uses 2000 for electronics)
    • Suggestion: Derive threshold from test data or make configurable
  6. Weighted average null handling

    • Issue: Division by items.size() when no weighted items exist
    • Suggestion: Implement proper fallback logic:
      BigDecimal avg = weightSum > 0 
          ? weightedSum.divide(BigDecimal.valueOf(weightSum), 2, RoundingMode.HALF_UP)
          : weightedSum.divide(BigDecimal.valueOf(items.size() > 0 ? items.size() : 1), 2, RoundingMode.HALF_UP);

Summary

Focus on fixing critical failing tests first by implementing requested immutability guarantees and proper edge case handling. The most important takeaway is to strictly match test specifications when implementing interface behaviors and validation rules.


Previous Feedback

🕒 Posted on 2025-10-22T09:09:49.306Z

Overall Feedback

The submission is mostly correct, addressing the core requirements with good structure and functionality. However, critical missing pieces prevent full functionality, particularly in the Warehouse class.

What's Working Well:

  1. WarehouseAnalyzer Implementation: Advanced methods (price range, expiration filters, analytics) are well-implemented.
  2. Product Hierarchy: FoodProduct and ElectronicsProduct correctly extend Product, implement interfaces, and handle validations (price/weight checks).
  3. Category Flyweight: Proper singleton pattern and name normalization are implemented.

Areas for Improvement:

  1. Missing getChangedProducts in Warehouse
    Issue: Assignment requires tracking changed products via getChangedProducts() but the method is absent.
    Fix: Add to Warehouse:

    public List<Product> getChangedProducts() {
         return new ArrayList<>(changedProducts); // Return unmodifiable copy
    }
  2. addProduct Condition Logic
    Issue: The duplicate ID check uses equals(Optional.empty()) which is valid but non-standard.
    Fix: Replace with clearer isPresent() check:

    if (getProductById(product.uuid()).isPresent()) {
         throw new IllegalArgumentException("Product with that id already exists, use updateProduct for updates.");
    }
  3. Weight Calculation Precision
    Issue: ShippingGroup uses Double.sum() on raw weights instead of converting BigDecimal. Test expectations assume BigDecimal precision.
    Fix: Use BigDecimal for summation:

    this.totalWeight = products.stream()
            .map(Shippable::weight)
            .map(BigDecimal::valueOf) // Convert Double to BigDecimal
            .reduce(BigDecimal.ZERO, BigDecimal::add).doubleValue();
  4. calculateWeightedAveragePriceByCategory Weight Handling
    Issue: The current code only considers weight > 0 when calculating average. Handling categories with all zero-weight products uses an arithmetic mean (invalid per test).
    Fix: Enforce weighted average for all categories:

    if (weightSum > 0) {
         avg = weightedSum.divide(BigDecimal.valueOf(weightSum), 2, RoundingMode.HALF_UP);
    } else {
         // Calculate simple arithmetic mean when no valid weights
         avg = weightedSum.divide(new BigDecimal(items.size()), 2, RoundingMode.HALF_UP);
    }

Summary

The implementation meets most requirements but requires critical additions to the Warehouse class and precision fixes in arithmetic calculations. Prioritize adding getChangedProducts() first.


Previous Feedback

🕒 Posted on 2025-10-20T12:18:10.703Z

Overall Feedback

LGTM! All implemented methods in WarehouseAnalyzer pass the supplied EdgeCaseTest suite. The solutions correctly handle edge cases, utilize proper rounding, and adhere to the assignment requirements. The only note is missing test coverage for findProductsAbovePrice, but the implementation aligns with the specification.

Key Strengths

  1. Robust Statistical Calculations: Implemented population standard deviation with correct outlier detection thresholds.
  2. Optimal Weighted Averages: Properly handled weighted vs. unweighted categories using product weights where available.
  3. Efficient Grouping Algorithms: First-fit decreasing approach minimizes shipping groups while obeying weight constraints.
  4. Precise Discount Logic: Correctly applied tiered expiration-based discounts with proper rounding.

Areas for Improvement

No significant issues found. The implementation is comprehensive and test-driven. All methods adhere to the interfaces and rounding specifications.


Previous Feedback

🕒 Posted on 2025-10-12T12:32:46.407Z

null


Previous Feedback

🕒 Posted on 2025-10-12T12:32:15.669Z

null


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants