Smalltalk - How to avoid typechecks in this situation?

Question

This is an example.

Assume the following constraints:

• A shop can only have 5 products.
• A food shop can only add Food products.
• A clothes shop can only add Clothes products.
• A mixed shop can add both products.

I have the following classes:

• AbstractShop

  • FoodShop
  • ClothesShop
  • MixedShop

• AbstractProduct

  • Clothing
  • Food

functionality:

    AbstractShop>>addProduct: aProduct
    (products size < 5)
    ifTrue:[^ products add:aProduct]
    ifFalse:[ self error: 'A shop can only have five products' ]

    FoodShop>>addProduct: aProduct
    (aProduct isMemberOf: Food)
    ifTrue:[^ super addProduct: aProduct]
    ifFalse:[ self error: 'You can only add food products to a foodshop' ]

    ClothesShop>>addProduct: aProduct
    (aProduct isMemberOf: Clothing)
    ifTrue:[^ super addProduct: aProduct]
    ifFalse:[ self error: 'You can only add clothing products to a clothes shop' ]

"ClothesShop doesn't override the addProduct, because it can add them both."

How can I avoid checking the type of the product, to see if it can be added to the shop? I want to avoid this, because it is a bad smell. I tried figuring it out with Double Dispatch, but it seems to make the code more difficult to maintain.

Answer 1

This is a pattern we have used several times. The class declares which things (or instances of which classes) can handle.

AbstractShop class >> allowedProducts
   self subclassResponsibility

FoodShop class >> allowedProducts
   ^#(Food)

ClothesShop class >> allowedProducts
   ^#(Clothing)

MixedShop class >> allowedProducts
   ^AbstractProduct subclasses

AbstractShop >> isAllowed: aProduct
   ^self class allowedProducts includes: aProduct class

 AbstractShop>>addProduct: aProduct
    products size < 5
       ifFalse:[ self error: 'A shop can only have five products' ].
    (self isAllowed: aProduct) 
       ifFalse: [self error: 'Product not allowed in this shop' ].
    ^ products add:aProduct

And you should not redefine addProduct.

But if smells say something, the magic constant 5 in the capacity check should also be extracted, and you should have the validation of the product separated from the add.

I never liked to call a class AbstractSomething, that is also a bad smell. In your case, using just Shop and Product is abstract enough.

Answer 2

You can also do this with double dispatch:

AbstractShop>>addProduct: aProduct
  (products size < 5)
    ifTrue:[ ^ aProduct addToShop: self ]
    ifFalse:[ self error: 'A shop can only have five products' ]

AbstractShop>>addFoodProduct: aProduct
  ^ self subclassResponsibility

AbstractShop>>addClothingProduct: aProduct
  ^ self subclassResponsibility

FoodShop>>addFoodProduct: aProduct
  ^ products add: aProduct

FoodShop>>addClothingProduct: aProduct
  self error: 'You can only add clothing products to a clothes shop'

AbstractProduct>>addToShop: aShop
  ^ self subclassResponsibility

Food>>addToShop: aShop
  ^ aShop addFoodProduct: self

Clothing>>addToShop: aShop
  ^ aShop addClothingProduct: self

Regarding your use case, I'd say that it's better to have a collection of supported items, a you can potentially have many shops with different combination of items. But with double dispatch you never check for types.