Commons Sandbox
  1. Commons Sandbox
  2. SANDBOX-416

Improve DFS/BFS visit detecting multiple states and related actions instead of just stop/continue

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Graph
    • Labels:
      None

      Description

      As discussed in ML, org.apache.commons.graph.visit.GraphVisitHandler methods that return boolean flags can be sometimes not so intuitive.

      The proposal is replacing boolean flags return statements with an enumeration values ABORT, CONTINUE, SKIP to identify

      • visit has to be immediately terminated
      • visit can continue;
      • current node children visit can be skipped.

        Activity

        Hide
        Claudio Squarcella added a comment -

        Hi,

        I just implemented and committed the change, see r1305162.
        I'll wait for your ack before marking the issue as resolved!

        Ciao,
        Claudio

        Show
        Claudio Squarcella added a comment - Hi, I just implemented and committed the change, see r1305162 . I'll wait for your ack before marking the issue as resolved! Ciao, Claudio
        Hide
        Simone Tripodi added a comment -

        Good morning!

        It looks very good, before resolving the issue I suggest to handle the ABORT state in every method - maybe the user had enough of visiting the graph and evaluated the visit is complete.

        When ABORT is detected, it just stop visiting (different from SKIP that should skip only the subtree) and invokes the onCompleted().

        TIA and well done,
        -Simo

        Show
        Simone Tripodi added a comment - Good morning! It looks very good, before resolving the issue I suggest to handle the ABORT state in every method - maybe the user had enough of visiting the graph and evaluated the visit is complete. When ABORT is detected, it just stop visiting (different from SKIP that should skip only the subtree) and invokes the onCompleted() . TIA and well done, -Simo
        Hide
        Claudio Squarcella added a comment -

        Hi Simone,

        I just committed the proposed change. Now ABORT is available also for the two methods discoveryEdge and discoveryVertex. Please see r1306016.

        If it looks good I guess we can close the issue.

        Cheers
        Claudio

        Show
        Claudio Squarcella added a comment - Hi Simone, I just committed the proposed change. Now ABORT is available also for the two methods discoveryEdge and discoveryVertex . Please see r1306016 . If it looks good I guess we can close the issue. Cheers Claudio
        Hide
        Simone Tripodi added a comment -

        very well done, I like the actual behavior

        Issue can be IMHO resolved, the only thing I would suggest you to improve is, when comparing values, putting expected values first, i.e. inetsad of

        if ( stateAfterEdgeDiscovery != VisitState.CONTINUE )
        

        it could be

        if ( VisitState.CONTINUE != stateAfterEdgeDiscovery )
        

        and constants could be statically imported. Not an issue, just a matter of having clean code.

        well done!
        -Simo

        Show
        Simone Tripodi added a comment - very well done, I like the actual behavior Issue can be IMHO resolved, the only thing I would suggest you to improve is, when comparing values, putting expected values first, i.e. inetsad of if ( stateAfterEdgeDiscovery != VisitState.CONTINUE ) it could be if ( VisitState.CONTINUE != stateAfterEdgeDiscovery ) and constants could be statically imported. Not an issue, just a matter of having clean code. well done! -Simo
        Hide
        Simone Tripodi added a comment -

        issue resolved according to r1306016 by Claudio - well done, mate!

        Show
        Simone Tripodi added a comment - issue resolved according to r1306016 by Claudio - well done, mate!
        Hide
        Claudio Squarcella added a comment -

        Hi,

        I saw you implemented the proposed improvements before I had the time to do so myself. Thanks for that!

        Ciao,
        Claudio

        Show
        Claudio Squarcella added a comment - Hi, I saw you implemented the proposed improvements before I had the time to do so myself. Thanks for that! Ciao, Claudio

          People

          • Assignee:
            Claudio Squarcella
            Reporter:
            Simone Tripodi
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development