2

I'm developing a page where users can see an overview of graphical images that represent Internet traffic coming in and going out of a network device's ports. Information about these ports are collected in a table. This table has at least the following columns per port:

  • Port ID
  • Device ID
  • PortName
  • and more...

The Device ID column is the primary key of another table: Devices. This table has at least the following columns:

  • Device ID
  • DeviceName
  • and more...

As you can see, there is a relation between these two tables by Device ID. I have translated both tables to Domain Objects 'Port' and 'Device'. Both Domain Objects get populated by two Domain Mappers 'PortMapper' and 'DeviceMapper'. One attribute I need from every port is the DeviceName which I can get through Device ID. My question is about how I can best map these two Domain objects together.

My PortMapper's findById() function looks like this:

public function findById($id, DeviceMapper $deviceMapper) {
    $ports = $this->db->sql_select('SELECT * FROM ports WHERE customer_id = ?', [$id]);

    $arrayOfPorts = [];

    foreach($ports as $port) {   
        $device = $deviceMapper->findById($port['device_id']);
        $port['device'] = $device;
        $arrayOfPorts[] = $this->createObject($port);
    }

    return $arrayOfPorts;
}

And my DeviceMapper's findById() function:

public function findById($id) {
    $device = $this->db->sql_select('SELECT * FROM devices WHERE device_id = ?', [$id]);
    return $this->createObject($device);        
}

The way I instantiate this is in my Controller:

$db = new Database('librenms');
$portMapper = new PortMapper($db);
$deviceMapper = new DeviceMapper($db);
$ports = $portMapper->findById($_SESSION['user']['id'], $deviceMapper);

foreach($ports as $port) {
    echo $port->getHostname();
}

I get the Device's hostname per Port just fine. But the way I instantiate the whole thing is really bad. What would be a better approach to the relation between two Domain Objects?

I can do a simple LEFT JOIN query so that I don't need to map two Domain Objects together, but that's not where I want to go to.

Beeelze
  • 475
  • 1
  • 5
  • 23

1 Answers1

1

I think you'll need to restructure a few things. First of all, I recommend that your data mapper populate your domain objects. After all, that's what their supposed to do: map data onto objects. This method allows you to set known attributes on a domain object, then call the data mapper to "fetch" the rest of the data, given the known variables.

The next thing I suggest is creating domain object collection classes. These entity collections are essentially glorified arrays and can act as such if implemented correctly. These changes will help in producing the relationships you want to map better (see code below). Additionally, with this advice, your mappers will assume a more singular responsibility, leaving other relational logic out of the mapper/in a helper class if you so choose.

Below I illustrate how I would go about structuring this task. First, a PortCollection is created and a condition on the ID is set. We can then map the PortCollection on a PortCollectionMapper. After making necessary database calls, the PortCollectionMapper sends the results to the PortCollection which instantiates new Port instances for every row fetched and adds it to the PortCollection.

Now, we can loop through the PortCollection and use a DeviceMapper to populate information about the device.

<?php

$portCollection = new PortCollection;
$portCollection->setId( $_SESSION['user']['id'] );

$portCollectionMapper = new PortCollectionMapper;
$portCollectionMapper->fetch( $portCollection );
//$portCollection is now populated with Port instances

$deviceMapper = new DeviceMapper;

foreach( $portCollection as $port )
{
    //create a device from the port device ID
    $device = new Device;
    $device->setId( $port->getDeviceId() );

    //populate the device with data
    $deviceMapper->fetch( $device );

    //assign the device to the port
    $port->setDevice( $device );
}
jeremy
  • 9,455
  • 4
  • 36
  • 57
  • I think it's useful to mention that `fetch` is using pass by reference. For the rest great example. – dbf Jun 11 '16 at 10:02
  • Thanks for this example. I have thought about using collections, but could not get my head around it for some reason. I should always take the Single Responsibility Principle in mind when writing code like this. – Beeelze Jun 11 '16 at 16:21
  • Would [this](https://www.sitepoint.com/handling-collections-of-aggregate-roots/) be a good tutorial to follow about handling collections? – Beeelze Jun 13 '16 at 07:29
  • @Jeremy I also used [another answer](http://stackoverflow.com/questions/17145798/handling-items-in-the-collection-pattern-by-a-data-mapper?rq=1) of yours. Very helpful! – Beeelze Jun 13 '16 at 08:38
  • Is it also possible to pass $portCollection to the constructor of $portCollectionMapper instead of passing it to fetch()? – Beeelze Jun 13 '16 at 11:14
  • @Beeelze yes, however you choose to implement, just make sure it won't cause problems for you later. – jeremy Jun 13 '16 at 12:26
  • @Jeremy Alright cool, i'll play around with it some more. I think you forgot to say that $portCollection class should implement IteratorAggregate? Otherwise you can not loop through it, is that correct? – Beeelze Jun 13 '16 at 12:28
  • @Beeelze yep, I left that out. collection classes should implement some type of iterator. you could also put a method on the collection classes like `getCollection()` – jeremy Jun 13 '16 at 12:30
  • @Beeelze I didn't see 2 of your comments earlier. I'm glad you found that answer helpful! ( upvote it ;)... though I did *not* use an iterator implementation there ) I did not read the SitePoint article, but the example code for collections definitely looks like something to consider when creating your collection. Read [here](http://stackoverflow.com/questions/11942842/who-should-handle-the-conditions-in-complex-queries-the-data-mapper-or-the-serv/11943107#11943107) also – jeremy Jun 19 '16 at 00:15
  • Is it correct to create a collection of objects, then loop through the collection to set more data/objects and when all is done loop through the whole collection again to use the data to create HTML tables for example? This sometimes means I loop through the whole thing 4 times. – Beeelze Jun 20 '16 at 06:50
  • @Beeelze hmmm I can't think of when that would be a good idea. Why not do it all in the same loop? – jeremy Jun 21 '16 at 12:12
  • @Jeremy Because then I would be generating HTML as well as collecting all information in one function, or all in the same class. This violates SRP imo. – Beeelze Jun 21 '16 at 12:23
  • @Beezle 1 loop to collect all information. Another loop for templating. That should be it – jeremy Jun 21 '16 at 15:16
  • Could we move this conversation to chat? – Beeelze Jun 22 '16 at 08:58
  • @Beeelze Do you mean you want to chat w/ me or just move these comments so they aren't on the answer? – jeremy Jun 22 '16 at 20:00
  • @Jeremy Both, I guess :-) – Beeelze Jun 23 '16 at 07:12
  • @Beeelze you can try to find me in the php room sometimes. otherwhise the people there are pretty smart (they know a lot more than i do on these topics). to move comments, flag for mod – jeremy Jun 23 '16 at 17:08