C#•2w ago

Verbosity to the point of unusability in generic graph class

I am trying to create a graph class, I'm using a lot of generics, it works fine but adding a weighted edge of weighted vertices is extremely unwieldy and basically unreadable.

            weightedGraph.AddEdge(new WeightedEdge<Foo, WeightedVertex<Foo, int>, int>(new WeightedVertex<Foo, int>(new Foo(), 0), new WeightedVertex<Foo, int>(new Foo(), 0), 0));

            weightedGraph.AddEdge(new WeightedEdge<Foo, WeightedVertex<Foo, int>, int>(new WeightedVertex<Foo, int>(new Foo(), 0), new WeightedVertex<Foo, int>(new Foo(), 0), 0));

This is insanely verbose but I have no idea how I could improve it without sacrificing type safety or versatility. For reference, the weighted edge and vertex classes:

    public class WeightedVertex<T, TWeight> : IVertex<T>
    {
        public T Value { get; set; }
        public TWeight Weight { get; set; }
        public WeightedVertex(T value, TWeight weight)
        {
            Value = value;
            Weight = weight;
        }
    }
    public class WeightedEdge<T, TVertex, TWeight> : IEdge<T, TVertex> where TVertex : IVertex<T>
    {
        public TVertex FirstVertex { get; set; }
        public TVertex SecondVertex { get; set; }
        public TWeight Weight { get; set; }

        public WeightedEdge(TVertex firstVertex, TVertex secondVertex, TWeight weight)
        {
            FirstVertex = firstVertex;
            SecondVertex = secondVertex;
            Weight = weight;
        }
    }

    public class WeightedVertex<T, TWeight> : IVertex<T>
    {
        public T Value { get; set; }
        public TWeight Weight { get; set; }
        public WeightedVertex(T value, TWeight weight)
        {
            Value = value;
            Weight = weight;
        }
    }
    public class WeightedEdge<T, TVertex, TWeight> : IEdge<T, TVertex> where TVertex : IVertex<T>
    {
        public TVertex FirstVertex { get; set; }
        public TVertex SecondVertex { get; set; }
        public TWeight Weight { get; set; }

        public WeightedEdge(TVertex firstVertex, TVertex secondVertex, TWeight weight)
        {
            FirstVertex = firstVertex;
            SecondVertex = secondVertex;
            Weight = weight;
        }
    }

and the graph class definition for a directed graph

    public class DirectedGraph<T, TVertex, TEdge> where TVertex : IVertex<T> where TEdge : IEdge<T, TVertex> {}

    public class DirectedGraph<T, TVertex, TEdge> where TVertex : IVertex<T> where TEdge : IEdge<T, TVertex> {}

30 Replies

canton7•2w ago

Presumably all edges and vertexes in a given graph have the same type? If so (and this one of the very few places where I'd advocate this), having Vertex and Edge by child classes of Graph would cut down on the number of generics flying around Some factory methods on weightedGraph would help a lot, maybe? Though just fixing the signature of AddEdge would also help Oh I see, you're allowed to substitute in any kind of edge and vertex. Why? And are you really instantiating 2 new vertexes every time you add a new edge?

BalthazarArgallOP•2w ago

Well, if you want a graph with mixed weighted and unweighted vertices of course! :D

canton7•2w ago

Your implementation doesn't let you mix weighted and non-weighted edges/vertexes, though?

BalthazarArgallOP•2w ago

Uh yes you're right I thought I was already at this step, let me re read that. You're saying I should have DirectedUnweighted, DirectedWeighted graphs etc.?

canton7•2w ago

Coming at this from an API standpoint, I would have thought that your API surface would be: * Add a new vertex * Add a new edge connecting two existing vertexes? In which case your methods are: * VertexType AddVertex(T value, TWeight weight) * EdgeType AddEdge(VertexType vertex1, VertexType vertex2, TWeight weight) I'm challenging how much complexity you want to have. Do you really need a user to be able to provide any sort of edge/vertex type they want? If not then yeah, having separate weighted/unweighted types would simplify the API a lot (they can still share a chunk of code -- you can still use your generics behind the scene)

BalthazarArgallOP•2w ago

What do you mean by "sort" of edge/vertex type? I only need the vertices and edges to either have weights or not

canton7•2w ago

As in, with your current impl, I could do new DirectedGraph<string, MyFancyVertex, MyFancyEdge>, where I've defined MyFancyVertex and MyFancyEdge myself. Do you need that?

BalthazarArgallOP•2w ago

Mh, no, indeed I don't plan on creating new types of vertices or edges, I didn;t include them but the only other class implementing IVertex is an Unweighted one and the only other class implementing IEdge is the same

canton7•2w ago

Then you don't need to expose that to the user IMO

public class WeightedDirectedGraph<TValue, TWeight>
{
    public WeightedVertex<TValue, TWeight> AddVertex(TValue value, TWeight weight);
}

public class WeightedVertex<TValue, TWeight>
{
}

public class WeightedDirectedGraph<TValue, TWeight>
{
    public WeightedVertex<TValue, TWeight> AddVertex(TValue value, TWeight weight);
}

public class WeightedVertex<TValue, TWeight>
{
}

BalthazarArgallOP•2w ago

Alright I see what you meant

canton7•2w ago

(You can still use your DirectedGraph<T, TVertex, TEdge> in the impl, to share code between the weighted and non-weighted graphs)

BalthazarArgallOP•2w ago

This doesn't allow the user to have weighted edges/unweighted vertices though

canton7•2w ago

That's true

BalthazarArgallOP•2w ago

Well I maybe go that route then, I'll sacrifice some versatility I guess

canton7•2w ago

Other thoughts: target-typed new can help weightedGraph.AddEdge(new(vertex1, vertex2, value))

BalthazarArgallOP•2w ago

Is that implicitly creating a class corresponding to what is asked by the function?

canton7•2w ago

It works out what to instantiate based on the signature of the method, yeah

BalthazarArgallOP•2w ago

Well, that worked

            weightedGraph.AddEdge(new(new(new Foo(), 0), new(new Foo(), 0),0));

            weightedGraph.AddEdge(new(new(new Foo(), 0), new(new Foo(), 0),0));

It looks...hard to parse though, would that be an acceptable fix?

canton7•2w ago

Yeah, I've no idea what's going on there

BalthazarArgallOP•2w ago

And I can't overload the method AddEdge now

canton7•2w ago

But it is really expected that you'd add 2 new edges and a new vertex at the same time? And new up the values at the same time?

BalthazarArgallOP•2w ago

Well, I need two vertices to create an edge, new or not

canton7•2w ago

Right, but if you're not newing them in that expression, it's easier to parse weightedGraph.AddEdge(new(vertex1, vertex2, new(value, 0))) You could also use extension methods:

public static void AddEdge<TValue, TVertex, TWeight>(this DirectedGraph<TValue, TVertex, WeightedEdge<TValue, TVertex, TWeight>>, TVertex vertex1, TVertex vertex2, TValue value, TWeight weight) where TVertex : IVertex<TValue> { ... }

public static void AddEdge<TValue, TVertex, TWeight>(this DirectedGraph<TValue, TVertex, WeightedEdge<TValue, TVertex, TWeight>>, TVertex vertex1, TVertex vertex2, TValue value, TWeight weight) where TVertex : IVertex<TValue> { ... }

Then you can do directedGraph.AddEdge(vertex1, vertex2, value, 0)

BalthazarArgallOP•2w ago

Alright gimme a moment Would that reside inside the graph class?

canton7•2w ago

That'd have to be in a separate static class

BalthazarArgallOP•2w ago

Not super keen on making this a static class unless I'm missing something, I was starting to think about factories to be honest

canton7•2w ago

As in, your extension methods need to be in a separate static class That's how extension methods work public static class DirectedGraphExtensions or something Then that can call directedGraph.AddEdge(new ...(vertex1, vertex2, new ...(value, weight)) And you can have different extension methods with different combinations of WeightedEdge, non-weighted Edge, etc. Intellisense will only suggest ones where the signature fits the type of DirectedGraph

BalthazarArgallOP•2w ago

Very well, I'll read up on extension method and try to implement that then Thank you for your time, really

canton7•2w ago

For problems like this, I like to start with designing the API, then work from there So figure out how the class should be used first. That's the bit that people care about.

Anton•2w ago

separating the builder part of the API from the graph class may also be a thing to consider

Gaming

Programming

Verbosity to the point of unusability in generic graph class