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

Wire class seems useless? #32

Open
jaipadmakumar opened this issue Aug 22, 2017 · 3 comments
Open

Wire class seems useless? #32

jaipadmakumar opened this issue Aug 22, 2017 · 3 comments

Comments

@jaipadmakumar
Copy link

Presumably, the concept of wire was initially introduced b/c, in a real circuit, wires connect gates. However, cello doesn't appear to use the Wire class at all. The structure of the graph is maintained by a Gate and not a Wire (Wire doesn't look to me like it's aware of anything).

I'm currently working on developing a set of tests for cello as a side project but bumping into a lot of difficulties b/c a lot of the classes are fairly tightly coupled, have functionality outside their purview, and there aren't any interfaces. As such, I'm starting w/ refactoring fairly large amounts of code. Should you agree with me on the Wire class, I'll also try to work on getting rid of it.

@tim-tx
Copy link
Contributor

tim-tx commented Aug 24, 2017

There do seem to be several references to Wire if you look at the output of grep -r 'Wire' src/main/java. Do you propose to eliminate all those in favor of keeping connectivity in Gate? I can't tell what's going on in the code just from grep but all the graph libraries I know of have separate node and edge classes, I might be in favor of moving the connectivity out of Gate and keeping Wire.

@jaipadmakumar
Copy link
Author

Something like that. This isn't as clear to me anymore as I thought it was when I first looked into doing this. I may have jumped the gun here...

Either of those two would, from my point of view, simplify the logic of the code. After taking a look again, it looks to me like the Wire is used to when Gate.getChildren() is called and that's what generally gets used for subsequent traversal. I guess that seems reasonable to me but I get confused b/c gates themselves can change positions and I think it forces a Gate to be dependent on a Wire in a sense.

Based on my limited understanding, an alternative from a future development and testing standpoint that may be good to consider is if we could somehow make a more general LogicCircuit to hold the structure of an arbitrary graph with indexed nodes and edges (ignoring the fact that it needs a Gate initially) and then place gates/wires 'on top' of those nodes/edges. That feels like it would help separate things a bit which would facilitate testing and make things less brittle, though it may end up adding code overall. For example, right now, I bet small changes to Gate would require a fairly large amount of work to make everything else play well.

I'm not sure what I suggested above would actually simplify things but it'd be nice if we could start to separate classes so they were less dependent on one another.

@Freyert
Copy link

Freyert commented Oct 13, 2017

It may be helpful to run a documentation initiative to better understand why classes exist. It may also make it easier to know which classes should be deprecated.

Getting more thorough Javadoc into this would help a bit.

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

No branches or pull requests

3 participants