oopdesign-patternsinterfaceclass-designobject-design

Making decisions on designing classes interfaces


I would like to get some thoughts from others about the following problem. Let's assume we have two classes Products and Items. Products object allows us to access any Item object. Here's the example.

$products = new Products();

// get existing item from products
$item = $products->get(123);

// create item
$item = $products->create();
$item->setName("Some new product");
$item->setPrice(2.50);

Now what would be the best way to update/save state of the item? I see 2 options:

$item->save();

or

$products->save($item);

First aproach seems very straigh forward. Once attributes of Item object are set calling save method on it will persist changes.

On the other hand I feel like latter approach is better. We're separating the roles of two objects. Item object contains only state and Products object operates on that state. This solution may be also better for writing unit tests. Any thoughts?


Solution

  • So, effectively the items are buffering the actual changes.

    Clearly both approaches will work, however it comes down to how closely you want to adhere to the underlying database's model or the overlaid object model.

    Viewed from the outside, $item->save() makes the most sense in terms of the model - as you point out, you update the item's properties and then save them down. Plus it is conceptually an action that is performed on the item.

    However, $products->save($item) offers two noticable advantages, and a drawback.

    On the plus side, by moving save into products, it can (potentially) handle batching / reordering of the updates in a smarter way since it has visibility of all the items. It also allows the save code to be used as ->add() (more or less)

    A downside is it is going to (from the object model view) add the following possible use, which you probably don't want:

    $p1 = new Products();
    $p2 = new Products();
    $item = $p1->create();
    // set $item values
    $p2->save($item);
    

    Obviously, you could just add an 'is this mine? no? then throw an error' test to Products::save, but that is extra code for blocking a use case the syntax implies could/should work. Or at least would probably slip through a code review.

    So, I'd say go with the approach that seems the simplest and binds tightest to the desired functionality ($item->save()), unless you need to do caching/batching/whatever that forces you to go with the other.