CIDARLAB / cello

Genetic circuit design automation
http://www.cellocad.org/
BSD 2-Clause "Simplified" License
808 stars 134 forks source link

Wire class seems useless? #32

Open jaipadmakumar opened 7 years ago

jaipadmakumar commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 6 years ago

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.