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

Theory#moore_dijkstra doesn't play nice with escaping #83

Closed
josephholsten opened this issue Nov 21, 2013 · 0 comments · Fixed by #84
Closed

Theory#moore_dijkstra doesn't play nice with escaping #83

josephholsten opened this issue Nov 21, 2013 · 0 comments · Fixed by #84

Comments

@josephholsten
Copy link
Contributor

Example script:

#!/usr/bin/env ruby

require 'graphviz'
require 'graphviz/theory'

graph = GraphViz.graph 'escaping-sut' do |g|
  g.add_nodes 'alice@example.com'
  g.add_nodes 'bob@example.com'
  g.add_edges 'alice@example.com', 'bob@example.com'
end

theory = GraphViz::Theory.new graph

route = theory.moore_dijkstra 'alice@example.com', 'bob@simplymeasured.com'

puts "alice -> bob distance: #{route[:distance]}"

Expected result:

alice -> bob distance: 1

Actual result

~/.rbenv/versions/2.1.0-preview1/lib/ruby/gems/2.1.0/gems/ruby-graphviz-1.0.9/lib/graphviz/theory.rb:249:in `block in distance_matrix': undefined method `index' for nil:NilClass (NoMethodError)
    from ~/.rbenv/versions/2.1.0-preview1/lib/ruby/gems/2.1.0/gems/ruby-graphviz-1.0.9/lib/graphviz.rb:233:in `block in each_edge'
    from /Users/joseph/.rbenv/versions/2.1.0-preview1/lib/ruby/gems/2.1.0/gems/ruby-graphviz-1.0.9/lib/graphviz.rb:232:in `each'
    from ~/.rbenv/versions/2.1.0-preview1/lib/ruby/gems/2.1.0/gems/ruby-graphviz-1.0.9/lib/graphviz.rb:232:in `each_edge'
    from /Users/joseph/.rbenv/versions/2.1.0-preview1/lib/ruby/gems/2.1.0/gems/ruby-graphviz-1.0.9/lib/graphviz/theory.rb:248:in `distance_matrix'
    from ~/.rbenv/versions/2.1.0-preview1/lib/ruby/gems/2.1.0/gems/ruby-graphviz-1.0.9/lib/graphviz/theory.rb:72:in `moore_dijkstra'
    from graphs/minimal.rb:15:in `<main>'

Which ends up coming from this line: https://github.com/glejeune/Ruby-Graphviz/blob/v1.0.9/lib/graphviz/theory.rb#L249

x = @graph.get_node(e.node_one( false )).index

Looking at the implementation of Edge#node_one, it is escaping the node id.

But looking at GraphViz#get_node, it's apparent that this function does not expect an escaped value.

A quick search implies that this method is only used in Edge#output, Theory#adjacency_matrix, Theory#incidence_matrix, Theory#degree, and Theory#distance_matrix.

I'd argue that none of the uses in Theory should have the escaping. I think it makes sense to push the escaping into Edge#output.

Does that sound good?

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

Successfully merging a pull request may close this issue.

1 participant