On Wed, Oct 6, 2010 at 10:32 AM, Markus Roberts <[email protected]>wrote:
> On Mon, Oct 4, 2010 at 10:24 AM, Paul Berry <[email protected]> wrote: > >> # Find a matching edge. Note that this only finds the first edge, > >> # not all of them or whatever. > >> def edge(source, target) > >> - @edges.each_with_index { |test_edge, index| return test_edge if > >> test_edge.source == source and test_edge.target == target } > >> + edge?(source,target) && @out_from[source][target].first > >> end > >> > >> def edge_label(source, target) > >> - return nil unless edge = edge(source, target) > >> - edge.label > >> + (e = edge(source, target)) && e.label > >> end > > > > It's hard to imagine circumstances in which the above two methods would > be > > useful, because if there are edges between the source and target, they > only > > return information about one of the edges. Also, it looks like nobody > calls > > these methods. Can we eliminate them? > > I started out determining the API by grepping our code base for all > the method names and eliminating those for which there were no static > calls. That didn't suffice (there were constructed calls) so I > switched to my wrapper & hot spot analysis leaving in anything I > hadn't proven unnecessary. That means I possibly left in dead code > but (if I didn't make any goofs) didn't remove any unneeded code. > > I have no objection to removing edge_lable, provided it really isn't > used anywhere. I'm more reluctant about edge since 1) the edges have > state the user may want to access and 2) if we don't provide an access > method (and don't reveal more of the internals than is prudent) to > user would need to do some pretty hackish / slow things to get > information we can provide cheaply and 3) it's just not that much > code. > > That said, if it isn't used rather I'd replace it with something like: > > def edges(source, target) > (edge?(source,target) && @out_from[source][target]) || [] > end > > or > > def edges(source, target) > ((@out_from[source] || {})[target]) || [] > end > As near as I can tell the only references to edge() and edge_label() were in spec tests. I'll go ahead and implement an edges() method along the lines you recommend (I'm calling it edges_between(), since an edges() method already exists), and I'll change the calls to edge_label() to use edges_between(). -- 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.
