Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I'm designing an OO graph library and at the moment I'm trying to figure out the design for a GraphEdge class. I've added setters and getters for it's nodes, direction and weight. This seemes perfectly reasonable.

Then I've started working on the Graph class and I've come to the need of knowing if the particular edge is adjacent to the particular node.

And now I doubt if I should add a isAdjacentTo(Node) method to the GraphEdge class or I should create a GraphUtil class(*) and add static edgeIsAdjacentToNode(Edge,Node) method to it.

On the one hand, I can not see any additional reasons for changing GraphEdge if I stick to the first option. On the other hand, this seems to me like kind of second responsibility added to the class.

Also if I stick to the first option this will probably violate the Open-Close Principle. The GraphEdge is not closed for changes, since later I will need to add linksNodes(Node,Node) method to it and maybe some others later.

The questions are:

  • Does adding isAdjacentTo(Node) method to the GraphEdge class violates the SRP?

  • Why?

(*) I stick to pure OO style with no free functions

share|improve this question
add comment

2 Answers 2

I believe you wandered in the field of overengineering. And chacing abstract principles instead of focusing on get things done. And done in the simplest and most pleasant way.

Your (*) is a clear giveaway. OO can provide great help in mabaging complexity. But only as long es it's used as a tool to that goal. If instead you want to "write pure OO" instead of "solving my problem", it becomes a burden.

SRP is a similar thing -- it's well worth thinking. And especially good use to drive "extract" or "split" type of refactorings when your current code is overburdened with too many tasks. After addressing DRY SPOT usually we still have a plenty of options on packaging, and it is a matter of balance. Too big class is bad, but having it single has benefits. If the same task is done by 5 classes it may be way harder to follow. Especially if none of them have a chance to work alone, they are coupled with cross-calls in all directions. Such split might look good on dumb statistics but is bad for practice anyway.

So much for the theory, what about your case. You picked a border problem, indeed the function may not fit either Node or Edge. Though even that is hard to tell without the content, knowing what those actually do. Can I walk the graph having just a Node or an Edge? If I can, then adjacent may fit to that same group of functions.

Actually as soon as you have implementation of the function, it should be a great help to see where it fits better -- what data it had to fetch. Say, if it starts with getting the node and edge collections from Graph, why on earth is it not just member of Graph and play on the home ground?

A free function might also be fair game. A *Utils fake class that is really just a trash bin is probably the worst way, that hardly wins anything but adds to complexity. But a language or an ill-conceived policy may force that.

share|improve this answer
    
what data it had to fetch the function uses only information stored in GraphEdge. It checks if one of the nodes equals the parameter. My GraphEdge stores pointers to both nodes inside itself. –  Kolyunya Jun 19 '13 at 11:04
    
In that case Node is already part of Edge and the function should just be there. I find the name somewhat odd, I'd probably call it hasNode or hasVertex. The most basic query of the state is hardly a new responsibility. –  Balog Pal Jun 19 '13 at 11:32
1  
@AmyBlankenship: Unfortunately we lack a right-o-meter so too many things are up to judgement. Also for most situations you have a matrix of pros&cons that can't be strictly compared for an absolute "better" –  Balog Pal Jun 19 '13 at 17:54
1  
and how a faked-free function is different? –  Balog Pal Jun 19 '13 at 22:29
1  
@AmyBlankenship: the question, "or I should create a GraphUtil class(*) and add static edgeIsAdjacentToNode(Edge,Node) method to it" –  Balog Pal Jun 20 '13 at 21:02
show 6 more comments

The answer is "neither." The problem with both your proposed solutions is that neither is an Information Expert for what it needs to know (how a single node relates to all the other nodes).

The only way you can have a free function that could calculate this information is by passing in all the nodes as an argument to the function. Otherwise, you do not have a free function, but brittle global state. If you already have all the nodes at your fingertips, do you really need the function?

More importantly, why doesn't the object that has all the nodes at its fingertips have this method? <--hint, hint

Now, you could conceivably have nodes that have access to this information, if they're implemented something like a doubly-linked-list, but with more directions than just next and previous. In this case, it would be trivial for a node to tell you if a given node is in its nextNode, previousNode, upNode, or downNode. But I suspect that you didn't code your nodes this way, or you wouldn't be asking the question. So go with the hint ;).

share|improve this answer
    
neither is an Information Expert for what it needs to know The edge IS an information expert for knowing if one of it's nodes equals some particular one or not. The question was about function checking if the particular edge links the particular node. –  Kolyunya Jun 20 '13 at 4:59
    
Btw, downvote not by me... –  Kolyunya Jun 20 '13 at 7:45
    
OK, I see now. I misread the question that the question was add it to the Node or a free function. The Edge would be the Class that has all the Nodes at its fingertips, as I said above. –  Amy Blankenship Jun 20 '13 at 17:11
    
Note that this would not violate SRP if the responsibility of an Edge is to manage a bunch of nodes, but, as @Balog Pal says, you haven't provided enough information to make it possible to determine what the responsibility of the Class is. –  Amy Blankenship Jun 20 '13 at 17:17
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.