2009-11-26 2 views
2

Я использую JGraphT, и я хочу, чтобы создать подграф, например:кода Java Обзор: Генерация подграф

Если есть график, подобный {stuff -> a -> b -> stuff}, и вы запрашиваете подграфа в направлении {a -> b}, вы» Получите {a -> b -> stuff}. (stuff = совокупность узлов и ребер, -> = край, a и b являются узлами.)

Моя идея заключается в следующем: если старт А, а раковина В, удалить все ребра из А, что дон 't привести к B. Затем, пересечь весь граф, и если никакой путь не существует из заданной вершины в A, удалите его.

Полный метод. Буду признателен за любые комментарии, отражающие любой метрики качества кода:

/** 
* subgraph that contains everything in the direction of sink 
* so if there's a graph like {stuff -> a -> b -> stuff} 
* and you ask for the subgraph in direction of {a -> b} 
* you'll get {a -> b -> stuff} 
* @param <V> vertex type 
* @param <E> edge type 
* @param g should be a dag 
* @param start a in the example above 
* @param sink b in the example above 
* @return a sub graph as specified above 
*/ 
public static <V, E extends DefaultEdge> Graph<V, E> subgraphInDirection(UndirectedGraph<V, E> g, final V start, final V sink) { 
    g = removeEdges(g, start, sink); 
    return removeUnconnectedNodes(g, start); 
} 

private static <Vertex, Edge> UndirectedGraph<Vertex, Edge> removeEdges(UndirectedGraph<Vertex, Edge> g, Vertex start, Vertex sink) { 
    final Set<Edge> outEdges = new HashSet<Edge>(g.edgesOf(start)); 
    boolean removedEdge; 

    for (Edge e : outEdges) { 
     if (! (g.getEdgeTarget(e).equals(sink) || g.getEdgeSource(e).equals(sink))) { 
      removedEdge = g.removeEdge(e); 
      assert removedEdge; 
     } 
    } 
    return g; 
} 

private static <Vertex, Edge> Graph<Vertex, Edge> removeUnconnectedNodes(UndirectedGraph<Vertex, Edge> g, Vertex start) { 
    ConnectivityInspector<Vertex, Edge> conn = new ConnectivityInspector<Vertex, Edge>(g); 
    boolean removedVertex; 

    final Set<Vertex> nodes = new HashSet<Vertex>(g.vertexSet()); 
    for (Vertex v : nodes) { 
     if (! conn.pathExists(start, v)) { 
      removedVertex = g.removeVertex(v); 
      assert removedVertex; 
     } 
    } 
    return g; 
} 

ОБНОВЛЕНО, чтобы отразить первоначальные комментарии.

+2

+1 для идеи сделать обзор кода для небольшого фрагмента на SO – abyx

ответ

2

Это должно было сработать:

Set<V> nodes = new HashSet<V>(g.vertexSet()); 
for (V v : nodes) { 
    if (! conn.pathExists(start, v)) { 
    g.removeVertex(v); 
    } 
} 

Это копирует узлы в новой коллекции и она должна устранить параллельные проблемы модификации.

+0

первоначально я делал 'Set nodes = g.vertexSet();'. Думаю, это не сработало бы, потому что не создало новый набор, он просто создает новую ссылку на существующую? –

+0

Да, это действительно решает проблему параллелизма. +1 –

1

Прежде всего, ваш javadoc неверен - либо удалите атрибуты, в которых вы ничего не написали, или не заполните детали.

Во-вторых, я бы разбил его на два отдельных метода, один для обработки краев, а другой для обработки вершин, что сделает код более понятным и более аккуратным.

Еще одна мелочь, имя removedE немного уродливое (IMHO), и я бы использовал что-то длиннее, чем просто E, например Edges.

Кроме этого, ничего не приходит в голову, и код выглядит хорошо для меня.

Относительно ConcurrentModificationException: Я не знаком с библиотекой, и поэтому не знаю, что это за ожидаемый результат, но обычно удаление элементов во время итерации вызывает проблемы, если только не существует специального метода для удаление элементов во время итерации для обновления Iterator (например, Iterator.remove).

Редактировать: После внесенных изменений код легче понять. Это может быть только я, но я бы извлек метод для длинного условного в первом методе к чему-то вроде isVertexConnectedToEdge().

Смежные вопросы