Assigning role responsibilities

[Matthew Browne]

Hi all,
I want to add a Dijkstra example to my DCI library for PHP. I was debating about how to design the functionality of finding the closest unvisited node from the start node, which is really a more general question: does it ever make sense to have a role that doesn't operate on its own role player's data, but only the data in other roles?

This is the role method in question, located in the UnvisitedNodes role in this version (let's call it version 1):

https://github.com/mbrowne/dci-php/blob/dijkstra/examples/Dijkstra/CalculateShortestPath.php#L193-L215

If I weren't thinking about the code, the mental model I would naturally lean toward would be to put that role method in StartNode, as I did on the following branch (version 2—and yes I know I did this backwards by not thinking more about my mental model up-front ;-) ):

https://github.com/mbrowne/dci-php/blob/dijkstra-start-node-method/examples/Dijkstra/CalculateShortestPath.php#L175-L196

trait StartNode

{

function findClosestUnvisitedNode() {

$this->context->currentNode->determinePreviousInPath();

$tentativeDistances = $this->context->tentativeDistances;

$unvisitedNodes = iterator_to_array($this->context->unvisitedNodes);

if (empty($unvisitedNodes)) {

return null;

}

return array_reduce(

$unvisitedNodes,

function($x, $y) use ($tentativeDistances) {

return $tentativeDistances->distanceTo($x) < $tentativeDistances->distanceTo($y)

? $x

: $y;

},

$unvisitedNodes[0]

);

}

}

As you can see from this code snippet, it doesn't use any of the data in the StartNode role player, but only CurrentNode, TentativeDistances, and UnvisitedNodes.

So it belongs to StartNode only based on my mental model (one of multiple possible mental models) and not any consideration about what data it's operating on. I'm leaning toward sticking with this mental model (StartNode.findClosestUnvisitedNode), but this is the first time I've really encountered this so I'd be curious to hear what you all think.

Thanks,
Matt

[James O Coplien]

On 3 Dec 2024, at 04.22, Matthew Browne <mbro...@gmail.com> wrote:

Hi all,
I want to add a Dijkstra example to my DCI library for PHP. I was debating about how to design the functionality of finding the closest unvisited node from the start node, which is really a more general question: does it ever make sense to have a role that doesn't operate on its own role player's data, but only the data in other roles?

This is the role method in question, located in the UnvisitedNodes role in this version (let's call it version 1):

https://github.com/mbrowne/dci-php/blob/dijkstra/examples/Dijkstra/CalculateShortestPath.php#L193-L215

I have found that having methods with arguments in DCI:

public function shortestPathFrom(Node $startNode, Node $destinationNode) 

might be a smell. Is there a reason that $startNode and $destinationNode are not Roles? They could be bound in the use case constructor. Maybe much of the rest of this method could go into the use case constructor.

Another smell: the method

private function processCurrentNode(): Node | null {

begs to be a method of the currentNode. I can see that it’s not because in your model, CurrentNode is not a role that a node can play. I think it’s a Role.

If I weren't thinking about the code, the mental model I would naturally lean toward would be to put that role method in StartNode, as I did on the following branch (version 2—and yes I know I did this backwards by not thinking more about my mental model up-front ;-) ):

I don’t think StartNode should have any methods. It is just another name for one of the objects. That object will be of interest when the code interacts with it through CurrentNode.

https://github.com/mbrowne/dci-php/blob/dijkstra-start-node-method/examples/Dijkstra/CalculateShortestPath.php#L175-L196

trait StartNode

{

function findClosestUnvisitedNode() {

$this->context->currentNode->determinePreviousInPath();

$tentativeDistances = $this->context->tentativeDistances;

$unvisitedNodes = iterator_to_array($this->context->unvisitedNodes);

if (empty($unvisitedNodes)) {

return null;

}

return array_reduce(

$unvisitedNodes,

function($x, $y) use ($tentativeDistances) {

return $tentativeDistances->distanceTo($x) < $tentativeDistances->distanceTo($y)

? $x

: $y;

},

$unvisitedNodes[0]

);

}

}

As you can see from this code snippet, it doesn't use any of the data in the StartNode role player, but only CurrentNode, TentativeDistances, and UnvisitedNodes.

Kay didn’t really believe in data; everything was a method to him. I wouldn’t sweat whether a given object’s data are manipulated through its role. I would go even further and say that I wouldn’t worry too much if a Role had no methods.

So it belongs to StartNode only based on my mental model (one of multiple possible mental models) and not any consideration about what data it's operating on. I'm leaning toward sticking with this mental model (StartNode.findClosestUnvisitedNode), but this is the first time I've really encountered this so I'd be curious to hear what you all think.

I don’t see anything wrong with that aspect of your mental model. I do have reservations about other aspects as I mention above. It looks too procedural and I think could be more DCI-like.

Thanks,
Matt

--
You received this message because you are subscribed to the Google Groups "object-composition" group.
To unsubscribe from this group and stop receiving emails from it, send an email to object-composit...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/object-composition/11b118db-5fd3-423c-a679-cb34c541c9c1%40gmail.com.

[Matthew Browne]

Hi Cope, thanks for the feedback.

On 12/3/24 4:50 AM, James O Coplien wrote:

I have found that having methods with arguments in DCI:

public function shortestPathFrom(Node $startNode, Node $destinationNode) 

might be a smell.

I just did it that way so that after creating an instance of CalculateShortestPath, you can calculate a shortest path again from a different start/destination in the same graph without having to create a new instance. I don't feel strongly about it and could certainly change it, but what is the downside of binding several roles in the shortestPathFrom() method (rather than the constructor), given that this Context has no other public operations?

Is there a reason that $startNode and $destinationNode are not Roles?

They are already roles in my implementation.

Another smell: the method

private function processCurrentNode(): Node | null {

begs to be a method of the currentNode. I can see that it’s not because in your model, CurrentNode is not a role that a node can play. I think it’s a Role.

CurrentNode was already a role as well, but I agree that the functionality in processCurrentNode() makes more sense as a role method. I just refactored it:

https://github.com/mbrowne/dci-php/blob/5ba98dc54bbc01f62ae3e4e30be050a57d2fa363/examples/Dijkstra/CalculateShortestPath.php#L124-L131

I don’t think StartNode should have any methods. It is just another name for one of the objects. That object will be of interest when the code interacts with it through CurrentNode.

Could you elaborate a bit on why that's your favored approach here?

Thanks,
Matt

[Lund Soltoft]

Den 4. dec. 2024 kl. 03.23 skrev Matthew Browne <mbro...@gmail.com>:

I just did it that way so that after creating an instance of CalculateShortestPath, you can calculate a shortest path again from a different start/destination in the same graph without having to create a new instance. I don't feel strongly about it and could certainly change it, but what is the downside of binding several roles in the shortestPathFrom() method (rather than the constructor), given that this Context has no other public operations?

Here’s the answer (slightly re-phrased) that I gave Trygve when we were debating this. If the context is a conversation between you, me and cope then at some point I leave and Richard join. Would you say it’s the same conversation/context or is it a new one. If you think it’s a new one then it follows it should have a new identity. If you think it’s the same conversation then rebinding is fine. 

To me rebinding suffers from some of the same mental issues as class oriented vs object oriented. It treats the context object as a compile time construct just as class oriented treats any type of objects. 

[James O Coplien]

Name me a responsibility of StartNode that is not already a responsibility either of CurrentNode or of nodes in general

Sendt fra min iPhone

> Den 4. dec. 2024 kl. 03.23 skrev Matthew Browne <mbro...@gmail.com>:
>

[Matthew Browne]

Hi Rune,

On 12/4/24 1:21 AM, Lund Soltoft wrote:

To me rebinding suffers from some of the same mental issues as class oriented vs object oriented. It treats the context object as a compile time construct just as class oriented treats any type of objects.

I can definitely see how role re-binding could lead to more fragile code in some cases, but in the case Cope was pointing out, it's all essentially initialization code. I think the point you've raised here might be a larger point than what Cope said, which are both worthy of discussion but we should probably separate those out.

In order to complete the Dijkstra algorithm with a single context instance, the CurrentNode role has to be re-bound. Alternatively it can be implemented using multiple context instances and recursion, as you did in your Marvin implementation of the algorithm.

I'm not strictly opposed to re-implementing it with immutable role bindings, but my original goal was just to write an implementation similar to the other existing DCI implementations with the exception of the Marvin one (as far as I know that's the only one that avoids role re-binding). And I would like to understand what the practical problem is in this particular case before doing so...to me it feels like I'm already thinking in terms of run-time, it's just that my run-time object happens to be mutable rather than immutable, kind of like how you can put new wheels on a car and it's still generally considered to be the same car. I can still create as many varying instances of the context as I want.

If I understand correctly, Cope was making a narrower point: not saying that no roles should be re-bound, but just that all the initial role bindings at the start of the algorithm should be done in the constructor rather than in CalculateShortestPath.shortestPathFrom(). But as I said above, I regard that as still being initialization code, or re-initialization in the case where you want to take an existing instance of CalculateShortestPath (where you already set the graph in the constructor) and now you want to know the shortest path between a different pair of nodes. It's a pretty minor convenience though (and yes I mean convenience, not inconvenience), and since it's a bit unconventional, maybe I should just move that role binding code to the constructor. I just don't see the actual downside of it.

Thanks,
Matt

[Matthew Browne]

On 12/4/24 4:25 AM, James O Coplien wrote:
> Name me a responsibility of StartNode that is not already a responsibility either of CurrentNode or of nodes in general

This question is exactly why I was uncertain about which role should
handle this responsibility... there are several objects that are all
involved in finding the closest unvisited node from the start node: the
current node, the set of unvisited nodes, the map of tentative
distances, and the start node. But the start node is only indirectly
involved; it just determines where the traversal starts, and all the
tentative distances are calculated relative to it.

So the mismatch was between "who" I intuitively thought should be
responsible for finding the closest unvisited node from the start (if I
didn't know the implementation details, my first inclination would be
"the start node"), and which objects are directly involved given how the
algorithm is actually implemented.

I already explored giving this responsibility to UnvisitedNodes
(UnvisitedNodes.findClosestFromStart()), which works—it's just less
intuitive to me, at least when I take off my coder hat.

But I just realized there's another way of thinking about it: instead of
thinking in terms of the next closest node from the start node, I could
think of it as the next closest node from the current node. This works
because of how the current node became the current node in the first
place: it was the next closest one from the start.

One other consideration is that the distance numbers in
TentativeDistances are all still relative to the start node. But despite
that, now I'm leaning toward making it
CurrentNode.findClosestUnvisitedNode().

It still seems like there's nothing terribly wrong with
StartNode.findClosestUnvisitedNode(), but I suppose that part of the
work of programming mental models is discovering ways that your model
can be refined and tweaked as you learn more details, so it's not
necessary to always stick 100% with what the upfront mental model
preference was.

[James O Coplien]

It “determines” (computes) nothing.

It has no responsibilities.

[James O Coplien]

Yup.

Sendt fra min iPhone

> Den 4. dec. 2024 kl. 14.31 skrev Matthew Browne <mbro...@gmail.com>:
>

[Raoul Duke]

€0.02

i know our programming ecosystems make it more laborious or sometimes impossible to 100% accomplish but i for one find it confusing and risky when "init" code is outside the constructor. 

i at least wish our languages made it easier to spit out Builder code as necessary. 

[James O Coplien]

Sounds like a job for ChatGPT.

Not.

--
You received this message because you are subscribed to the Google Groups "object-composition" group.
To unsubscribe from this group and stop receiving emails from it, send an email to object-composit...@googlegroups.com.

To view this discussion visit https://groups.google.com/d/msgid/object-composition/CAJ7XQb7rsHuVcC1KwmZqfJ7OwYWku1NJ_bFE%3DOwx_tdroAd0EA%40mail.gmail.com.

[Matthew Browne]

Update: I moved those role assignments to the constructor, because I realized I hadn't actually tested calling the same context instance multiple times, and making that work correctly would have required binding UnvisitedNodes and ShortestPathSegments in the operation method (shortestPathFrom()), which definitely would have put it over the edge to where it was doing too much.

I also moved StartNode.findClosestUnvisitedNode() to CurrentNode.findClosestUnvisitedNode(). Here's the updated version in case anyone is interested:

https://github.com/mbrowne/dci-php/blob/540eb5e0f032c5bf02adbf01843b5b175771cfcf/examples/Dijkstra/CalculateShortestPath.php

BTW, I kept the original street map with its weights from the original examples on fulloo.info, because I realized (when I first started working on this) that only uneven weights make a difference for the shortest path when it's a grid with only perpendicular intersections. So the weights here mean traffic or some other factor, since higher weights for larger distances wouldn't be meaningful for a city grid.

To view this discussion visit https://groups.google.com/d/msgid/object-composition/6EC8F737-2DF6-4DAF-8AA3-62C358F0D9B8%40gmail.com.

[James O Coplien]

On 8 Dec 2024, at 02.50, Matthew Browne <mbro...@gmail.com> wrote:

BTW, I kept the original street map with its weights from the original examples on fulloo.info, 

O.K., I’ll bite. Where on fulloo.info?

[Matthew Browne]

In the C++ example, for one:

https://fulloo.info/Examples/C++Examples/Dijkstra/DCIDijkstraInC++.html

But I just remembered that I did tweak the numbers a bit, even though the letters and their arrangement is the same, and the shortest path is the same. (This is what happens when you start working on something and then put it down for a few months before actually finishing it, haha.) I remember now that I initially changed the avenues to all have a cost of at least 3, since avenues in Manhattan are as long as 3 streets. But it doesn't really have any consequences on how it works.

FYI I also looked at the trygve example as a starting point, which uses the same street grid as the C++ one, and technically that's linked from fulloo.info too but only indirectly via Github—I just noticed that it isn't actually listed on this page, in case you wanted to add it: https://fulloo.info/Examples/TrygveExamples/

[James O Coplien]

aha, tx.