Skip to content
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

Interaction potential graphs should be restructured #353

Open
jbphet opened this issue Apr 5, 2021 · 0 comments
Open

Interaction potential graphs should be restructured #353

jbphet opened this issue Apr 5, 2021 · 0 comments

Comments

@jbphet
Copy link
Contributor

jbphet commented Apr 5, 2021

I've noticed this before, but it became painfully apparent while working on #352. The inheritance hierarchy for the various interaction potential graphs make the code very hard to maintain, and there is inappropriate coupling between the various levels of the hierarchy. Much of this is on me, since I allowed it to pass code review, but I shouldn't have. I'm generally the one who pays the price by having to navigate these oddities, so I guess that's karma in action.

Honestly, I don't know if it will ever become a high enough priority to address this, but it will have to be done if we ever want to reuse these graphs in another simulation. I'll try to make improvements whenever I have to touch the code, as I just did, but what it really needs is a thoughtful rewrite.

I guess I'm logging this mostly as a signpost to any future would be users of the code for the interaction potential graphs: Here be dragons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant