0

I'm coding an editor for graphs (Graph Theory).Let's imagine vertex needs these properites :

class Vertex{
int ID {get;}
Color color {get; set;}
Point point{get; set;}
}

But it's violating of SRP(single responsibility principle). So I created somethink like :

class Vertex{
int ID {get;}
}

class Positions{
private Dict<Vertex,Point> _pos;

setPosition(Vertex v, Point pos);
Point getPosition(Vertex v);
}

//etc.

Right?

But ViewModel for vertex needs, all of these properties to be displayed.

class VertexVM
{
Vertex _v;
Positions _positions;
//...
Point position
{
     get {return _positions.getPosition(_v); }
}

// same for color etc

}

Is it violationg of SRP? (In my opinion, it is.) Is there any way how to avoid it? Thanks.

bidacek
  • 101
  • 1
  • 6
  • 1
    I struggle with this constantly. My personally preferred approach (not the only approach either) is to use partial classes for my complex view models. – Backlash Mar 26 '13 at 17:35
  • SRP doesn't mean you should have classes containing only a single property... – walther Jun 07 '13 at 19:19
  • @walther Yes, I know... It's defined by reasons to change. But I can definitely name two reasons to change this class(Position 2D -> 3D , Color RGB -> HSL for example) – bidacek Aug 09 '13 at 09:41
  • Maybe try to find what does "a reason to change" actually mean then :) You're still modifying a property of a class, it's a single reason. If SRP meant what you think it means, you'd have only one-property classes. Use the common sense when designing your classes or you'll face numerous problems later on. Principles aren't a law, they're guidelines and should be treated as such. – walther Aug 09 '13 at 17:09
  • @walther : I still think the class, which manages colors (it could be at least five converting methods) and positions (i can imagine at least for methods too), violates SRP. I know it's not a big deal and it won't be the end of the world, if i write it this way. I actually needed the answer for a theoretic part of my thesis. – bidacek Sep 11 '13 at 20:41

1 Answers1

0

I would say your initial vertex definition is violating SRP, but not for the reason you seem to imply by your re-factoring. On the other hand, your VertexVM is violating SRP.

A vertex in graph theory is a point or location within the graph. It by definition should be responsible for knowing its location. Therefore it should contain the point. Whether Color belongs in it or not is a different case. I'm guessing maybe not as this is likely more display related and doesn't belong in a model class. A vertex type that results in the view displaying the vertex in a particular color might be appropriate.

In VertexVM, the View Model is intended to represent a single Vertex to the view. It is not its responsibility to know the positions of all the vertices in the graph. This definitely violates SRP.

brader24
  • 485
  • 2
  • 10
  • I'm pretty sure, that vertex isn't defined by it's location. I know plenty of graph theory problems, which don't involve the positions of vertices (adjacency matrix is valid graph). – bidacek Aug 09 '13 at 09:56
  • I can't unfortunately see any violation of SRP, when VertexVM asks Positions(Model layer) about its position. Please, correct me if I am wrong. Thanks. – bidacek Aug 09 '13 at 10:15
  • If you are using Vertex to refer to a vertex in a 2D plane where the color has meaning to the vertex in the model and not just how it is displayed, then I believe the original vertex definition is sufficient. If, however, you later want to allow it to be in a 3D plane or have a number value instead of a color, then changing it to support both of these directly would violate SRP. I still believe knowledge of positions for all vertices, violates SRP for VertexVM. – brader24 Aug 09 '13 at 12:02
  • OK, I get it now. I was only trying to explain, that the position isn't any special property. DFS alg - no color, no position; Coloring alg - no positions, only colors; Kruskal alg - no colors, only positions I agree it's weird that the VertexVM has reference to Positions (which holds position of all vertices), but it can only access values of its model vertex (the VertexVM doesn't know, there are some other vertices). Thanks a lot. I've sorted out my thoughts. – bidacek Aug 09 '13 at 15:32