Hacker News new | ask | show | jobs
by fransje26 795 days ago
I must be misunderstanding something from this article. With:

struct Node {

    std::vector<Connection> connections;
};

struct Connection {

    Node* from, to;
};

Does this mean that to create the vector of connections, Nodes are created, and references are taken to store in the Connections? And then the Nodes are stored in the list, with std::move()?

I don't understand why you would want to go down that road. Intuitively, I would assume that you are not safe from an object copy somewhere down the line and your graph then comes crashing down like a house of cards. Wouldn't it make more sense to store the nodes as pointers? If you like to live dangerously, something like:

struct Graph {

    std::vector<std::list<Node\*>> nodes;
};

Or better:

struct Graph {

    std::vector<std::list<std::unique_ptr<Node>>> nodes;
};

The later will give you plenty of warnings if you do not copy Nodes around with std::move().

Or less performant, but maybe safer, std::shared_ptr<Node>, together with:

struct Connection {

    std::weak_ptr<Node> from, to;
};

so that you have some check guarantees before access?

1 comments

I don't think you're misunderstanding, it's a strange choice to make even if the example here loses context to make it easier to make the point. It wasn't code that anyone currently here wrote and because it worked with the old compiler, nobody really touched it. I believe the nodes are pushed into the list before adding connections but I don't think that changes the point you're trying to make. That said, one's intuition is different from another's and I don't think it's unfair to assume that pushing into a vector of lists won't cause every existing list to be copied. For that to be the actual behavior is kind of disgusting. It may make more sense to create pointers here but that's a larger change to deal with and ensure correctness versus just swapping the vector out for another list. I don't claim to like that solution but it seems to me like legacy C++ code in general is fragile enough as it is, the less I have to change to fix bugs the better.
Thank you for the clarification!

> It may make more sense to create pointers here but that's a larger change to deal with and ensure correctness versus just swapping the vector out for another list. I don't claim to like that solution but it seems to me like legacy C++ code in general is fragile enough as it is,

That's more than fair enough. In such code, more often than not, you make what you think is a small change, and you end up with an entire cascade of unexpected side-effects that pop-up because the original assumptions and the testing scope of those assumptions were lost. A can of worms best left unopened.

By the way, wouldn't such an error be something that gets detected by the "new" ASAN functionality that has been added to the newer MSVC toolchain you are using?