Skip to content

Conversation

@moueedAli
Copy link

No description provided.

@ninja-aruna
Copy link

The design follows a clear object-oriented approach with well-defined classes and responsibilities.
Areas for Improvement

  1. Class Relationships
    • The class diagram shows inheritance between MemberPublic and Customer, but this might not be the best approach. A Customer IS-A MemberPublic, but in practice, composition might be better since a customer has additional behaviours rather than being a specialized type of member.
    Suggestion: Consider making Customer contain a MemberPublic rather than inherit from it.
  2. Basket Capacity Logic
    • The capacity check is currently shown in MemberPublic, but it might belong more naturally in the Basket class itself.
    Suggestion: Move capacity-related checks to the Basket class with methods like isFull() and setCapacity().
  3. Item Management
    • The Items class is quite generic. You might want to consider having a Bagel class and Filling class that extend or implement Items, as they might have different properties.
  4. Inventory Management
    • The Manager class handles inventory, but there's no clear way to add/remove items from inventory. Consider adding inventory management methods.
    Specific Recommendations
    For Basket Class:
  5. Add methods:
    o isFull(): Returns boolean if basket is at capacity
    o getRemainingCapacity(): Returns how many more items can be added
    o setCapacity(int newCapacity): With proper validation
  6. Consider making the capacity final or adding validation to prevent negative capacities.
    For Items Class:
  7. Consider making this an abstract class or interface with concrete implementations:
    public interface Item {
    String getSKU();
    double getPrice();
    String getName();
    String getVariant();
    }

public class Bagel implements Item { ... }
public class Filling implements Item { ... }
For Manager Class:

  1. Add inventory management methods:
    o addToInventory(Item item)
    o removeFromInventory(Item item)
    o isInInventory(Item item)
    For MemberPublic/Customer:
  2. The addBagel and removeItem methods might be better placed in the Basket class, with the public/customer classes just calling those methods.
    Potential Missing Requirements
  3. No mention of how pricing works for bagels with multiple fillings.
  4. No clear way to view the current basket contents.
  5. No way to clear the entire basket at once.

@moueedAli
Copy link
Author

  1. Are you applying that Customer class should extend MemberPublic class? Or that Customer class should have an instance of MemberPublic?
  2. Sounds good
  3. Yeah, considering to make it an interface instead
  4. Yes, on it

@ninja-aruna
Copy link

  1. Are you applying that Customer class should extend MemberPublic class? Or that Customer class should have an instance of MemberPublic?
  2. Sounds good
  3. Yeah, considering to make it an interface instead
  4. Yes, on it

The Problem with Inheritance Here
In your current design where Customer extends MemberPublic, you're implying that:

A Customer is-a MemberPublic (inheritance relationship)

All Customers automatically get all MemberPublic behaviors

This creates several potential issues:

Inheritance is rigid - What if you need a Customer that isn't a MemberPublic?

Future changes become difficult - Changing MemberPublic affects all Customers

Multiple roles problem - What if someone is both a Manager and Customer?

Over-inheritance - Customers inherit all MemberPublic methods even if irrelevant

Why Composition Works Better
Instead, having Customer contain a MemberPublic means:

A Customer has-a MemberPublic (composition relationship)

More flexible and maintainable design

`public class MemberPublic {
private String name;
private Basket basket;

public MemberPublic(String name) {
    this.name = name;
    this.basket = new Basket();
}

// Basic member methods
public void addBagel(Bagel bagel) { /* ... */ }
public void removeItem(Item item) { /* ... */ }

}

public class Customer {
private MemberPublic memberPublic;
private PaymentMethod paymentMethod;
private OrderHistory orderHistory;

public Customer(MemberPublic memberPublic) {
    this.memberPublic = memberPublic;
}

// Customer-specific methods
public double getTotalCost() { /* ... */ }
public void applyLoyaltyDiscount() { /* ... */ }

// Delegate to memberPublic when needed
public void addBagel(Bagel bagel) {
    memberPublic.addBagel(bagel);
}

}`

@moueedAli
Copy link
Author

I think there might have been a misunderstanding, probably because of my class diagram or domain model. But the idea I have is that they are distinct classes, so Customer and MemberPublic are their own classes, without anything in common. Can I just go with that approach? I don't quite understand why a customer needs to have an instance of MemberPublic. Is it just implying that a customer can also be a member of public?

@ninja-aruna
Copy link

ninja-aruna commented Aug 11, 2025

I think there might have been a misunderstanding, probably because of my class diagram or domain model. But the idea I have is that they are distinct classes, so Customer and MemberPublic are their own classes, without anything in common. Can I just go with that approach? I don't quite understand why a customer needs to have an instance of MemberPublic. Is it just implying that a customer can also be a member of public?

Yea, the idea is that a customer can also be a member of the public, and a manager can be a customer, etc. But we can stick with your approach of having unrelated classes

@moueedAli
Copy link
Author

Regarding the methods calculateTotalCostOfBasket and showCostOfItemBeforeAdding, are these methods that should be in the Basket class? Then let the customer call these methods from their class through a basket instance?

@ninja-aruna
Copy link

Regarding the methods calculateTotalCostOfBasket and showCostOfItemBeforeAdding, are these methods that should be in the Basket class? Then let the customer call these methods from their class through a basket instance?

calculateTotalCostOfBasket - I would put this in the basket c;lass
showCostOfItemBeforeAdding - this feels like it belongs in the inventory/stock class

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.

2 participants