Refactor topo.py:class Edge(object) #25

Closed
jvimal opened this Issue Feb 14, 2012 · 3 comments

Comments

Projects
None yet
3 participants
Owner

jvimal commented Feb 14, 2012

I really don't like the laundry list of parameters, especially the ones we added later on (enable_ecn, enable_red, use_hfsc, ...). It looks really really bad, though it's explicit. Also, maintaining that constructor is error prone!

A cleaner implementation would be to add a kwargs object to the constructor, and use something like __getattr__. The downside is bad readability---someone browsing the code would have no idea what options to pass. I think it's not a big deal, as we can document it.

Any thoughts?

Owner

brandonheller commented Feb 14, 2012

Someone browsing the code would have a good idea if this were documented properly. That's the easiest sane way to do this.

Owner

lantz commented Feb 14, 2012

I think the "right" way to do it may be to simply augment the existing Topo classes to allow specification of:

  • default Host, Switch and Link classes or constructors (note: we need to create a Link class)
  • default parameters for same
  • optionally per-node/per-link classes and parameters

This could be "simplified" further by currying parameters into the constructors, although I don't know if that's actually better.

A key point is that the Host/Switch/Link parameters don't belong in Topo - they belong in the Host/Switch/Link classes, where they can and should be documented.

I have been thinking about this for a while and can create a sample implementation.

Owner

lantz commented May 13, 2012

Implemented in cs244 branch.

lantz closed this May 13, 2012

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