OOP Decision Maker Class - ServerArbiter

Question

I am writing a ServerArbiter, the job of which is to make a decision about which server to "choose" given a set of rules. The aim is to be able to add / remove rules at will - for ease of future maintenance. My language of choice is php, so please bear this in mind when answering.

Basically, a server will be chosen depending on various factors. One may be 'server load'. Imagine this arbiter is used to decide which server to add a torrent to - it will choose the server with the least number of torrents in the download queue.

Architecture

My current architecture for this solution consists of a ServerArbiter object:

public function __construct(RuleSet $ruleSet, array $servers);

The RuleSet is effectively just a list of Rule objects that implements \Iterator so it can be foreach()d around.

public function addRule(Rule $rule);

And the actual Rule objects implement the Rule interface:

public function resolve(Server $server);

A Server is just a (doctrine) Entity - that is, it contains class members, setters and getters, and some validation. These properties are populated with data from the servers database table. One of the properties is ipAddress.

Rules

I currently have rules such as the ServerActiveRule, which is just a property that is returned.

class ServerActiveRule implements Rule
{
 /**
 * {@inheritdoc}
 */
 public function resolve(Server $server)
 {
 return $server->isActive();
 }
}

For torrents example described above, I was going to have a ServerDownloadsComparisonRule:

class ServerDownloadsComparisonRule
{
 /**
 * @var Server[]
 */
 private $servers;
 public function __construct(array $servers)
 {
 // -- SNIP --
 // Make sure each $server is instance of Server entity
 // -- SNIP --
 }
 public function resolve(Server $server)
 {
 // I'm guessing this is where I will compare the downloads to get the one with the least number of downloads in
 // I haven't implemented this yet
 }
}

ServerArbiter::decide()

Finally, this is the current method I have of deciding which Server entity to return.

/**
 * Run each Server against each Rule in the RuleSet and return the Server that matched the most rules
 *
 * @return Server The one Server to rule them all
 *
 * @note If two servers match the same number of rules successfully, the first one will be returned
 */
public function decide()
{
 $decisions = [];
 /** @var Rule $rule */
 foreach ($this->ruleSet as $rule)
 {
 foreach ($this->servers as $server)
 {
 if (!array_key_exists($server->getId(), $decisions))
 {
 $decisions[$server->getId()]['successes'] = 0;
 }
 if ($rule->resolve($server))
 {
 $decisions[$server->getId()]['successes']++;
 }
 }
 }
 $decision = array_search(max($decisions), $decisions);
 return $decision;
}

Questions

1. Simple rules that return true / false are fine. But as soon as ServerDownloadsComparisonRule gets thrown into the mix, it has a different constructor requiring an array of servers. This spells mayhem for the ServerArbiter::decide() method as it's now doing a lot more work. How can I refactor all this to make it much simpler to let each rule individually do what it needs to do?

2. Obviously my rule for checking downloads will have to connect to the server via it's IP address. Is it okay to Inject a dependency to perform this data retrieval just for the rule?

3. How is the architecture, what can I do differently? What is stupid about it?

4. Is there anything else that I can do better?

Extra Info

I'm using a recursively instantiating dependency injection container. This means I can typehint for any object anywhere in my application, including in this service, and the object will automatically be passed in for me. Therefore, I don't have to worry about wiring - it needs to be more SOLID and simple to modify in the future.

• Marek: I think you should allow for hard rules, and preference. Return true or false if server must be or must not be used, respectively, integer to increase or decrease the preference.

Answer 1

1. Why does the decide() method do more work? It's the problem of the object constructing your Rules, which is not necessarily the ServerArbiter.
2. Isn't that already a part of the Server entity? Why do you need to inject yet another dependency. If the server's address isn't in the Server entity, why the hell the server's address isn't in the Server entity?!
3. Your architecture is fine. Easily separated, Servers don't know about the Rules.

4. Yes. Since querying all the servers to get their downloads count, it makes sense to remember the best server in the ServerDownloadsComparisonRule the first time resolve is called, and then just reference it from the cache.

   if ($bestServer == null) {
    $bestServer = $this->findServerWithLeastDownloads();
   }
   return $server === $bestServer;
   

• James: The downloads are from another server (the ipAddress is a property of the server, though), and therefore are not part of the Server entity. In order to get those downloads, a call over HTTP will have to be made. I was under the impression that putting a getDownloads() method with the relevant HTTP call code would be giving the Entity too much to do? Caching-wise, the downloads could change on each server any second, so to get the 'best' server would need to be calculated each time the best server is requested.
• James: HTTP-wise, I was going to have a service dedicated to getting the downloads, and all you would pass in is an IPAddress. That's it - SRP.
• Madara's Ghost: That would be the part of the mapper then. Because it's an expensive action, I'd make it a Service action to actually populate the array of servers with their downloads.
• Madara's Ghost: Let's continue this discussion in chat