Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add general purpose Attributes, replace Layoutable #103

Merged
merged 7 commits into from
Dec 31, 2014
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/Fhaculty/Graph/Algorithm/MaximumMatching/Flow.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ public function getEdges()
// create temporary flow graph with supersource and supersink
$graphFlow = $this->graph->createGraphCloneEdgeless();

$superSource = $graphFlow->createVertex()->setLayoutAttribute('label', 's*');
$superSink = $graphFlow->createVertex()->setLayoutAttribute('label', 't*');
$superSource = $graphFlow->createVertex();
$superSink = $graphFlow->createVertex();

$groups = $alg->getGroups();
$groupA = $groups[0];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ public function createGraph()
// create resulting graph with supersource and supersink
$resultGraph = $this->graph->createGraphClone();

$superSource = $resultGraph->createVertex()->setLayoutAttribute('label', 's*');
$superSink = $resultGraph->createVertex()->setLayoutAttribute('label', 't*');
$superSource = $resultGraph->createVertex();
$superSink = $resultGraph->createVertex();

$sumBalance = 0;

Expand Down
34 changes: 34 additions & 0 deletions lib/Fhaculty/Graph/Attribute/AttributeAware.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php

namespace Fhaculty\Graph\Attribute;

/**
* Implemented by any entity that is aware of additional attributes
*
* Each attribute consists of a name (string) and an arbitrary value.
*/
interface AttributeAware
{
/**
* get a single attribute with the given $name
*
* @param string $name
* @return mixed|null
*/
public function getAttribute($name);

/**
* set a single attribute with the given $name to given $value
*
* @param string $name
* @param mixed $value
*/
public function setAttribute($name, $value);

/**
* get a container for all attributes
*
* @return AttributeBag
*/
public function getAttributeBag();
}
34 changes: 34 additions & 0 deletions lib/Fhaculty/Graph/Attribute/AttributeBag.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php

namespace Fhaculty\Graph\Attribute;

/**
* Interface to container that represents multiple attributes
*/
interface AttributeBag extends AttributeAware
{
// public function getAttribute($name);
// public function setAttribute($name, $value);
// public function getAttributeBag();

/**
* set an array of additional attributes
*
* @param array $attributes
*/
public function setAttributes(array $attributes);

/**
* get an array of all attributes
*
* @return array
*/
public function getAttributes();

/**
* get an array of the names of all existing attributes
*
* @return string[]
*/
public function getNames();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not yet decided how useful a getNames() method actually is. It causes a bit of code duplication and it's the only method without "attribute" in its name.

Personally, I'd tend towards dropping it completely, but I'd love to hear some other opinions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. array_keys($bag->getAttributes()) seems useful enough to me?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely, I guess it's safe to just drop this altogether.

}
42 changes: 42 additions & 0 deletions lib/Fhaculty/Graph/Attribute/AttributeBagContainer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

namespace Fhaculty\Graph\Attribute;

class AttributeBagContainer implements AttributeBag
{
private $attributes = array();

public function getAttribute($name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding a $default = null parameter to this method could be useful. That would allow the user to override the value returned if the attribute doesn't exist. Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes perfect sense, thanks for the suggestion!

{
return isset($this->attributes[$name]) ? $this->attributes[$name] : null;
}

public function setAttribute($name, $value)
{
$this->attributes[$name] = $value;

return $this;
}

public function getAttributes()
{
return $this->attributes;
}

public function setAttributes(array $attributes)
{
$this->attributes = $attributes + $this->attributes;

return $this;
}

public function getNames()
{
return array_keys($this->attributes);
}

public function getAttributeBag()
{
return $this;
}
}
71 changes: 71 additions & 0 deletions lib/Fhaculty/Graph/Attribute/AttributeBagNamespaced.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<?php

namespace Fhaculty\Graph\Attribute;

/**
* An attribute bag that automatically prefixes a given namespace
*/
class AttributeBagNamespaced implements AttributeBag
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice idea!

{
private $bag;
private $prefix;

public function __construct(AttributeAware $bag, $prefix)
{
if (!($bag instanceof AttributeBag)) {
$bag = $bag->getAttributeBag();
}
$this->bag = $bag;
$this->prefix = $prefix;
}

public function getAttribute($name)
{
return $this->bag->getAttribute($this->prefix . $name);
}

public function setAttribute($name, $value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The set methods in this class don't return $this for the fluent interface like the other implementations of AttributeBag do. Is that on purpose? Should the fluent interface be part of the AttributeAware and AttributeBag interfaces?

{
$this->bag->setAttribute($this->prefix . $name, $value);
}

public function getAttributes()
{
$attributes = array();
$len = strlen($this->prefix);

foreach ($this->bag->getAttributes() as $name => $value) {
if (strpos($name, $this->prefix) === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't this unintentionally remove a prefix if the prefix is purposefully included multiple times using nested attribute bags? For example, commit.commit.sha.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how this could happen? Care to elaborate or perhaps even provide a test case? :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you are correct. I didn't quite grasp the implementation here the first time through, but it looks correct even in weird cases like:

$bag = new AttributeBagContainer();
$commitBag = new AttributeBagNamespaced($bag, 'commit.');
$commitDetailBag = new AttributeBagNamespaced($commitBag, 'commit.');
// ...

I do believe that something like this could cause problems. They aren't critical problems, but they do allow people to get around the namespacing:

$bag = new AttributeBagContainer();
$bag->setAttribute('commit.sha', 'abc123');

$commitBag = new AttributeBagNamespaced($bag, 'commit.');
$commitBag->getAttribute('sha'); // 'abc123'

$attributes[substr($name, $len)] = $value;
}
}

return $attributes;
}

public function setAttributes(array $attributes)
{
foreach ($attributes as $name => $value) {
$this->bag->setAttribute($this->prefix . $name, $value);
}
}

public function getNames()
{
$names = array();
$len = strlen($this->prefix);

foreach ($this->bag->getAttributes() as $name => $value) {
if (strpos($name, $this->prefix) === 0) {
$names []= substr($name, $len);
}
}

return $names;
}

public function getAttributeBag()
{
return $this;
}
}
47 changes: 47 additions & 0 deletions lib/Fhaculty/Graph/Attribute/AttributeBagReference.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

namespace Fhaculty\Graph\Attribute;

class AttributeBagReference implements AttributeBag
{
private $attributes;

public function __construct(array &$attributes)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this by ref. This will be confusing. Say

$ar['color'] = 'red';
$a_attr = new AttributeBagReference($a);
$ar['color'] = 'blue';
$b_attr = new AttributeBagReference($a);

if ($a_attr->getAttribute('color') == $b_attr->getAttribute('color'))
{
    echo "Weird";
}

Or did I miss something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AttributeBagRef is designed to be a lightweight temporary instance so that we can expose the attributes through the AttributeBag interface without actually storing the instance. I.e. every time you call $vertex->getAttributeBag() you get another instance (which is just an implementation detail), but you can still access all properties nonetheless.

The AttributeBagContainer has it's own internal set of attributes and could be used as an alternative to the AttributeBagRef. Using it instead would require every Vertex to have (or at least lazy-load) a dedicated instance and then implement it like this:

class Vertex
{
    public function getAttribute($name)
    {
        return $this->getAttributeBag()->getAttribute($name);
    }
}

I suspected this to be inefficient (additional instances, more references, forwarded method calls etc.), but we should probably run some benchmarks first.

{
$this->attributes =& $attributes;
}

public function getAttribute($name)
{
return isset($this->attributes[$name]) ? $this->attributes[$name] : null;
}

public function setAttribute($name, $value)
{
$this->attributes[$name] = $value;

return $this;
}

public function getAttributes()
{
return $this->attributes;
}

public function setAttributes(array $attributes)
{
$this->attributes = $attributes + $this->attributes;

return $this;
}

public function getNames()
{
return array_keys($this->attributes);
}

public function getAttributeBag()
{
return $this;
}
}
22 changes: 20 additions & 2 deletions lib/Fhaculty/Graph/Edge/Base.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace Fhaculty\Graph\Edge;

use Fhaculty\Graph\Layoutable;
use Fhaculty\Graph\Vertex;
use Fhaculty\Graph\Set\Edges;
use Fhaculty\Graph\Set\Vertices;
Expand All @@ -13,8 +12,10 @@
use Fhaculty\Graph\Exception\UnderflowException;
use Fhaculty\Graph\Exception\InvalidArgumentException;
use Fhaculty\Graph\Exception\BadMethodCallException;
use Fhaculty\Graph\Attribute\AttributeAware;
use Fhaculty\Graph\Attribute\AttributeBagReference;

abstract class Base extends Layoutable implements VerticesAggregate
abstract class Base implements VerticesAggregate, AttributeAware
{
/**
* weight of this edge
Expand All @@ -40,6 +41,8 @@ abstract class Base extends Layoutable implements VerticesAggregate
*/
protected $flow = NULL;

protected $attributes = array();

/**
* get Vertices that are a target of this edge
*
Expand Down Expand Up @@ -289,4 +292,19 @@ private function __clone()
throw new BadMethodCallException();
// @codeCoverageIgnoreEnd
}

public function getAttribute($name)
{
return isset($this->attributes[$name]) ? $this->attributes[$name] : null;
}

public function setAttribute($name, $value)
{
$this->attributes[$name] = $value;
}

public function getAttributeBag()
{
return new AttributeBagReference($this->attributes);
}
}
26 changes: 20 additions & 6 deletions lib/Fhaculty/Graph/Graph.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
use Fhaculty\Graph\Set\VerticesMap;
use Fhaculty\Graph\Set\Edges;
use Fhaculty\Graph\Set\DualAggregate;
use Fhaculty\Graph\Attribute\AttributeAware;
use Fhaculty\Graph\Attribute\AttributeBagReference;

class Graph implements DualAggregate
class Graph implements DualAggregate, AttributeAware
{
/**
* @var ExporterInterface|null
Expand All @@ -36,6 +38,8 @@ class Graph implements DualAggregate
protected $edgesStorage = array();
protected $edges;

protected $attributes = array();

public function __construct()
{
$this->vertices = VerticesMap::factoryArrayReference($this->verticesStorage);
Expand Down Expand Up @@ -100,7 +104,7 @@ public function createVertexClone(Vertex $originalVertex)
}
$newVertex = new Vertex($this, $id);
// TODO: properly set attributes of vertex
$newVertex->setLayout($originalVertex->getLayout());
$newVertex->getAttributeBag()->setAttributes($originalVertex->getAttributeBag()->getAttributes());
$newVertex->setBalance($originalVertex->getBalance());
$newVertex->setGroup($originalVertex->getGroup());

Expand All @@ -117,7 +121,7 @@ public function createVertexClone(Vertex $originalVertex)
public function createGraphCloneEdgeless()
{
$graph = new Graph();
// $graph->setLayout($this->getLayout());
$graph->getAttributeBag()->setAttributes($this->getAttributeBag()->getAttributes());
// TODO: set additional graph attributes
foreach ($this->getVertices() as $originalVertex) {
$vertex = $graph->createVertexClone($originalVertex);
Expand Down Expand Up @@ -236,7 +240,7 @@ private function createEdgeCloneInternal(Edge $originalEdge, $ia, $ib)
$newEdge = $a->createEdge($b);
}
// TODO: copy edge attributes
$newEdge->setLayout($originalEdge->getLayout());
$newEdge->getAttributeBag()->setAttributes($originalEdge->getAttributeBag()->getAttributes());
$newEdge->setWeight($originalEdge->getWeight());
$newEdge->setFlow($originalEdge->getFlow());
$newEdge->setCapacity($originalEdge->getCapacity());
Expand Down Expand Up @@ -497,8 +501,18 @@ public function __toString()
return $this->getExporter()->getOutput($this);
}

public function getLayout()
public function getAttribute($name)
{
return isset($this->attributes[$name]) ? $this->attributes[$name] : null;
}

public function setAttribute($name, $value)
{
$this->attributes[$name] = $value;
}

public function getAttributeBag()
{
return array();
return new AttributeBagReference($this->attributes);
}
}
Loading