-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
collection of suggestions from reddit and own ones #2
Comments
I agree with the reddit comments. I'm going through fixing some memory leaks I found using testing.allocator right now. I'll also look at the style guide soon.
|
There are 2 ways: 1. keep it separated from the
Either this or they handle data separately. Your graphdata shows what an result could look like.
Yes, though this is the responsibility of the user. Its common to use partially invalid graphs as iterators to generate the new graph structure. If you delete edges, the user must copy the structure.
Yes. This may not be wanted, so it should be documented that its the users responsibility to check, if an edge |
|
https://git.skewed.de/count0/graph-tool was the link (so I can refer to it later). Keep in mind to first make alot of tests of edge cases against memory leak stuff. |
Hey @sournav, how are you doing? Did you manage to debug things so far etc? |
Good, thanks for asking, you? My bad I got a little caught up with finals, I'll be working on this for the comming week. I should just have to finish adding comments in order to resolve this issue. Then I'll move to algos. |
I am also busy with my thesis. No problem. First things first and good luck with the finals. Mitchell also became busy again (https://github.com/mitchellh/zig-graph). |
Good luck with your thesis! Cool, another fellow zig grapher! Okay so I should be done with the general comments, and the rest of the issues we discussed as well. I will make a wiki with some of the specific design decisions, but overall I think we can close this issue if you don't have any additional comments. I'll be adding more features as I need to if it makes writing the graph algos easier. |
Reddit comments
Personal comments
graphdata
andgraph
you useAddNode
, but typically the user writes his own wrapper for the data manipulation and you only provide a way to "comptime derive the interface". For now as "simple start, lol" to show what the user might want this works.RemoveNodeUndirected
andRemoveNodeDirected
. It may look nice, but this may break user experience ie when people want to have a broken graph to connect it themself for a special method etc. KISS and YAGNI until stuff is polished or move it to a concrete example instantiation of the graph.orderedRemove
Sorry for the numerous points. Graph stuff is hard.
The text was updated successfully, but these errors were encountered: