Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Graph edgeSize is not always correct #4

Closed
maloku opened this issue Jun 28, 2013 · 1 comment
Closed

Graph edgeSize is not always correct #4

maloku opened this issue Jun 28, 2013 · 1 comment
Labels

Comments

@maloku
Copy link

maloku commented Jun 28, 2013

I think there is a Bug by decreasing edgeSize after removing multiple nodes at the same time. edgeCount of nodes has incorrect value after deleting a neighbor node.

The following test fails:

describe "Another tricky test -- ",->
  it "edgeSize after adding and removing nodes",->
    graph = new Graph();
    expect(graph.addNode "1").toBeDefined()
    expect(graph.addNode "2").toBeDefined()
    expect(graph.addNode "3").toBeDefined()
    expect(graph.addNode "4").toBeDefined()
    expect(graph.addNode "5").toBeDefined()
    expect(graph.addEdge "1", "2").toBeDefined()
    expect(graph.addEdge "2", "4").toBeDefined()
    expect(graph.addEdge "2", "3").toBeDefined()
    expect(graph.addEdge "2", "5").toBeDefined()
    expect(graph.addEdge "5", "4").toBeDefined()
    expect(graph.addEdge "1", "3").toBeDefined()
    ###
    1 -> 2 ->  4
     \   | \   ^
      \  |  \  |
       ˅ ˅   ˅ |
         3     5
    ###

    expect(graph.removeNode "2").toBeDefined()
    expect(graph.removeNode "1").toBeDefined()
    ###
          4
          ^
          |
          |
    3     5
    ###
    expect(graph.edgeSize).toBe 1 ## Bug edgeSize = 0
    expect(graph.nodeSize).toBe 3
@chenglou
Copy link
Owner

Argh, thanks a lot, you've discovered a conceptual bug here. I was striving to make removeNode O(1) amortized, but finally it's too cumbersome to implement for no practical benefit. Switched it back to traditional O(E).
The upside is that the code's much cleaner.

It's on npm now!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants