Csson / p5-DBIx-Class-Visualizer

Visualize a DBIx::Class schema using graphviz generated SVG
Other
2 stars 2 forks source link

Proposed changes so far #4

Open mohawk2 opened 3 years ago

mohawk2 commented 3 years ago

Some suggested changes. I'm just at the point of making a lazy-built Graph object (attribute as_graph), and will then use it with a graphvizify function to pass to GraphViz2->from_graph, as discussed. These changes all seemed like a good idea on the way. You'll notice I've turned graph from a full-on attribute, to being the constructor-arg for a new graphviz property, so that graph can be lazy-built, which meant being able to get rid of BUILD. I also moved a couple of functions/methods around. The use of Test::Snapshot makes it easy to spot any changes to the produced DOT file, which should avoid subtle errors.

What do you think?

Csson commented 3 years ago

Wow! This is going to take some time to digest, not least since I haven't worked deeply with this code for the last couple of years. I'll work on this over the coming weeks.

mohawk2 commented 3 years ago

Hi! Good/bad news - there's more now. A guide: the idea is to switch this library from a DBIC-like set of classes, to being graph-centred. With the latest addition, there's now an as_graph method that produces an idiomatic graph that describes the schema. It just knows about outward edges, and doesn't do the matching-up. The docs and test snippets ought to give a picture on how that works.

There's now also a graphvizify function (not method) that takes such a graph, copies it, then mutates that copy to make it show nicely in graphviz (by adding graphviz attributes that interpret the attributes such as columns/primary/foreign key info, or for edges the from_key etc). It does any matching of mutually matching edges, and disfavours belongs_to, same as your code.

A benefit of separating these things is that one could take the return value of graphvizify and add a groups key under graphviz (see the docs) after to highlight certain tables, and/or delete the nodes (tables) in a more flexible way before, or use Graph's new subgraph_by_radius (which now can take multiple vertices, to mimic or indeed implement your wanted/degrees_of_separation functionality.

This fairly thorough both because it's an interesting problem, and because it's a good exercise of Graph, and GraphViz2 (revealing some now-fixed shortcomings of both those), and because these techniques can be used for wider use in e.g. updating SQL::Translator::Producer::GraphViz, doing automated analyses of schemas for e.g. normalisation, etc.

Let me know your thoughts!