2

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?

marcin_koss
  • 5,763
  • 10
  • 46
  • 65
  • Seems a little confused what exactly is save doing? Are you implying that Products stores some additional data about the item (so save is updating the Product object's state) or that it is triggering Products to perform some action upon the item, or that setname/setprice are being transactioned in some way and save will apply the changes? – Oliver Matthews May 14 '14 at 15:15
  • @OliverMatthews Products does not store additional information about the item. Products->save will decide to create new records or update the record. setName, setPrice just set values to objects attributes that's all. – marcin_koss May 14 '14 at 15:22
  • 3
    $item->save() is an example of the ActiveRecord pattern and $products->save($item) is an example of the TableGateway pattern. See http://martinfowler.com/eaaCatalog/activeRecord.html and http://martinfowler.com/eaaCatalog/tableDataGateway.html to discover the pros and cons of each. There are other data access patterns hat you might want to look at there as well. – Jason Nesbitt May 14 '14 at 15:29

1 Answers1

1

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.

Oliver Matthews
  • 7,497
  • 3
  • 33
  • 36
  • What are your thoughts about maintainability? I have a feeling having database operations in one class and data we're operating on in another may make things a bit easier. Also regarding your example with $p1 and $p2, maybe singleton pattern would be a good use in here? – marcin_koss May 20 '14 at 13:11
  • If the database access could be reasonably declared to be a singleton, then implementing that as a separate, third class and referenced from within `$item-save()` and `$products->create()` would nicely pull it out, but would be a nightmare to refactor should you ever decide you need multiple connections. – Oliver Matthews Jun 09 '14 at 08:44