> I'm going to try to come up with a revised version of the patch this week
> that addresses the issues Jesse and I came up with in the code review.
> I'll pay particular attention to speed testing the downstream_from_vertex()
> and upstream_from_vertex() methods so that we don't wind up unduly
> sacrificing speed for readability.

For some Fortranesque definition of "readability"  :)

You may be able to find something mutually satisfactory (for some
definition of satisfactory) by pulling some of the sub-expressions out
as separate functions such as:

  def downstream_neighbors_of(v)
    @out_from[v].keys
  end

  def at_and_downstream_from_vertex(v)
    downstream_neighbors_of(v).inject({}) { |result,node|
result.update(node=>1).update downstream_from_vertex(node) }
  end

  def downstream_from_vertex(v)
    @downstream_from[v] ||= ...
  end

but the real problem may be that "upstream from" and "downstream from"
are not quite the concepts we want, and that's what's making them
awkward to code acceptable (readable, efficient, etc.).  The two key
issues I found myself struggling with were 1) it's the painful sort of
transitive closure (< vs. <=) since we don't want to include v and 2)
using memoization to avoid the need for loop detection is a
performance win but prevents the use of ||= for memoization.

-- M
-----------------------------------------------------------
The power of accurate observation is
commonly called cynicism by those
who have not got it.  ~George Bernard Shaw
------------------------------------------------------------

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to