给定一个有向图,请找到从源到目标的所有路径。

查找代码复习,优化和最佳实践。还需要帮助弄清楚复杂性,在我的最佳尝试中,这是O(E!),其中E是边的数量。

class GraphFindAllPaths<T> implements Iterable<T> {

    /* A map from nodes in the graph to sets of outgoing edges.  Each
     * set of edges is represented by a map from edges to doubles.
     */
    private final Map<T, Map<T, Double>> graph = new HashMap<T, Map<T, Double>>();

    /**
     *  Adds a new node to the graph. If the node already exists then its a
     *  no-op.
     * 
     * @param node  Adds to a graph. If node is null then this is a no-op.
     * @return      true if node is added, false otherwise.
     */
    public boolean addNode(T node) {
        if (node == null) {
            throw new NullPointerException("The input node cannot be null.");
        }
        if (graph.containsKey(node)) return false;

        graph.put(node, new HashMap<T, Double>());
        return true;
    }

    /**
     * Given the source and destination node it would add an arc from source 
     * to destination node. If an arc already exists then the value would be 
     * updated the new value.
     *  
     * @param source                    the source node.
     * @param destination               the destination node.
     * @param length                    if length if 
     * @throws NullPointerException     if source or destination is null.
     * @throws NoSuchElementException   if either source of destination does not exists. 
     */
    public void addEdge (T source, T destination, double length) {
        if (source == null || destination == null) {
            throw new NullPointerException("Source and Destination, both should be non-null.");
        }
        if (!graph.containsKey(source) || !graph.containsKey(destination)) {
            throw new NoSuchElementException("Source and Destination, both should be part of graph");
        }
        /* A node would always be added so no point returning true or false */
        graph.get(source).put(destination, length);
    }

    /**
     * Removes an edge from the graph.
     * 
     * @param source        If the source node.
     * @param destination   If the destination node.
     * @throws NullPointerException     if either source or destination specified is null
     * @throws NoSuchElementException   if graph does not contain either source or destination
     */
    public void removeEdge (T source, T destination) {
        if (source == null || destination == null) {
            throw new NullPointerException("Source and Destination, both should be non-null.");
        }
        if (!graph.containsKey(source) || !graph.containsKey(destination)) {
            throw new NoSuchElementException("Source and Destination, both should be part of graph");
        }
        graph.get(source).remove(destination);
    }

    /**
     * Given a node, returns the edges going outward that node,
     * as an immutable map.
     * 
     * @param node The node whose edges should be queried.
     * @return An immutable view of the edges leaving that node.
     * @throws NullPointerException   If input node is null.
     * @throws NoSuchElementException If node is not in graph.
     */
    public Map<T, Double> edgesFrom(T node) {
        if (node == null) {
            throw new NullPointerException("The node should not be null.");
        }
        Map<T, Double> edges = graph.get(node);
        if (edges == null) {
            throw new NoSuchElementException("Source node does not exist.");
        }
        return Collections.unmodifiableMap(edges);
    }

    /**
     * Returns the iterator that travels the nodes of a graph.
     * 
     * @return an iterator that travels the nodes of a graph.
     */
    @Override public Iterator<T> iterator() {
        return graph.keySet().iterator();
    }
}

/**
 * Given a connected directed graph, find all paths between any two input points.
 */
public class FindAllPaths<T> {

    private final GraphFindAllPaths<T> graph;

    /**
     * Takes in a graph. This graph should not be changed by the client
     */
    public FindAllPaths(GraphFindAllPaths<T> graph) {
        if (graph == null) {
            throw new NullPointerException("The input graph cannot be null.");
        }
        this.graph = graph;
    }

    private void validate (T source, T destination) {

        if (source == null) {
            throw new NullPointerException("The source: " + source + " cannot be  null.");
        }
        if (destination == null) {
            throw new NullPointerException("The destination: " + destination + " cannot be  null.");
        }
        if (source.equals(destination)) {
            throw new IllegalArgumentException("The source and destination: " + source + " cannot be the same.");
        }
    }

    /**
     * Returns the list of paths, where path itself is a list of nodes.
     * 
     * @param source            the source node
     * @param destination       the destination node
     * @return                  List of all paths
     */
    public List<List<T>> getAllPaths(T source, T destination) {
        validate(source, destination);

        List<List<T>> paths = new ArrayList<List<T>>();
        recursive(source, destination, paths, new LinkedHashSet<T>());
        return paths;
    }

    // so far this dude ignore's cycles.
    private void recursive (T current, T destination, List<List<T>> paths, LinkedHashSet<T> path) {
        path.add(current);

        if (current == destination) {
            paths.add(new ArrayList<T>(path));
            path.remove(current);
            return;
        }

        final Set<T> edges  = graph.edgesFrom(current).keySet();

        for (T t : edges) {
            if (!path.contains(t)) {
                recursive (t, destination, paths, path);
            }
        }

        path.remove(current);
    }    

    public static void main(String[] args) {
        GraphFindAllPaths<String> graphFindAllPaths = new GraphFindAllPaths<String>();
        graphFindAllPaths.addNode("A");
        graphFindAllPaths.addNode("B");
        graphFindAllPaths.addNode("C");
        graphFindAllPaths.addNode("D");

        graphFindAllPaths.addEdge("A", "B", 10);
        graphFindAllPaths.addEdge("A", "C", 10);
        graphFindAllPaths.addEdge("B", "D", 10);
        graphFindAllPaths.addEdge("C", "D", 10);

        graphFindAllPaths.addEdge("B", "C", 10);
        graphFindAllPaths.addEdge("C", "B", 10);

        FindAllPaths<String> findAllPaths = new FindAllPaths<String>(graphFindAllPaths);
        List<List<String>> paths = new ArrayList<List<String>>();

        // ABD
        List<String> path1 = new ArrayList<String>();
        path1.add("A"); path1.add("B"); path1.add("D");

        // ABCD
        List<String> path2 = new ArrayList<String>();
        path2.add("A"); path2.add("B"); path2.add("C"); path2.add("D");

        //ACD 
        List<String> path3 = new ArrayList<String>();
        path3.add("A"); path3.add("C"); path3.add("D");

        //ABCD
        List<String> path4 = new ArrayList<String>();
        path4.add("A"); path4.add("C"); path4.add("B"); path4.add("D");

        paths.add(path1);
        paths.add(path2);
        paths.add(path3);
        paths.add(path4);

        findAllPaths.getAllPaths("A", "D");

        assertEquals(paths, findAllPaths.getAllPaths("A", "D"));
    }
}


评论

您可能应该指定是否假设没有循环。这不是一个小细节。

它处理周期

您的解决方案几乎是完美的。但是,您应该在递归方法中使用current.equals(destination)。

#1 楼

这是美丽,易于阅读且文档齐全的代码。阅读此代码很高兴,而Java方面没有什么改进。

该代码中显示的最大弱点不是您对Java的熟练程度(远远超过了我的能力),而是您对英语的了解。


在JavaDoc addEdge中,您谈论的是弧而不是边。可以是“源和目标都应为非空”或“源和目标都应为非空”。
该方法还具有非常令人困惑的JavaDoc“如果长度为if,则为@param长度” –我不是确定这是什么意思。

但是让我们继续讲技术方面。



recursive方法中有一个不错的错误:您正在通过T比较==实例:

if (current == destination) {


但是,这默认为比较指针。它仅在这里起作用,因为常量字符串"A""B"等由Java实现池合并。除非比较身份是您真正想要的,否则请进行比较:current.equals(destination)

关于recursive将忽略循环的评论是错误的–它只会构造每个节点最多访问一次的路径。这是处理循环的唯一正确方法,否则循环图中将存在无数的路径。

来自validate的错误消息没有任何意义。

if (source == null) {
    throw new NullPointerException("The source: " + source + " cannot be  null.");


因此,我们将收到错误消息“源:null不能为null”。这没有帮助,我建议:

if (source == null) {
    throw new NullPointerException("The \"source\" parameter cannot be  null.");



在您的测试代码中,您经历了太多麻烦而无法产生预期的paths。我非常喜欢Arrays.asList(…)

List<List<String>> paths = Arrays.asList(
    Arrays.asList("A", "B", "D"),
    Arrays.asList("A", "B", "C", "D"),
    Arrays.asList("A", "C", "D"),
    Arrays.asList("A", "C", "B", "D")
);


该代码简洁且具有自我说明性。除了将路径指定为代码外,还将路径放入注释中,当注释在ABCD路径上方的ACBD处出现时,这是错误的; 。路径的顺序应该被认为是无关紧要的(您实际上对路径集合感兴趣),并且未指定,因为使用HashMap不能保证密钥的保留顺序。实际上,您正在遍历keySet(),它将确定路径的顺序!更好的相等性测试可能是:

assertEquals(new HashSet<List<String>>(paths), new HashSet<List<String>>(result));




评论


\ $ \ begingroup \ $
路径是LinkedHashSet
\ $ \ endgroup \ $
– JavaDeveloper
2014年3月29日18:40

\ $ \ begingroup \ $
@JavaDeveloper对此感到抱歉,我现在删除了该段。不知道我在想什么
\ $ \ endgroup \ $
–阿蒙
2014年3月29日18:50

\ $ \ begingroup \ $
很高兴我编码,因为您会给我反馈,进一步肯定了我的信心
\ $ \ endgroup \ $
– JavaDeveloper
2014年3月30日在5:53

\ $ \ begingroup \ $
在平面设计评审提案FYI中引用。
\ $ \ endgroup \ $
–rolfl
2014年6月16日下午16:46

#2 楼

只是一些小事:



这里的注释不正确,应该是ACBD



//ABCD
List<String> path4 = new ArrayList<String>();
path4.add("A"); path4.add("C"); path4.add("B"); path4.add("D");



您可以使用Guava的newArrayList(或编写类似的方法)从测试代码中删除一些重复项:

List<String> path4 = newArrayList("A", "C", "B", "D");



此处无需打个电话:


findAllPaths.getAllPaths("A", "D");

assertEquals(paths, findAllPaths.getAllPaths("A", "D"));




为此:


if (graph.containsKey(node)) return false;



根据Java编程语言的代码约定,


if语句始终使用大括号{}。


容易出错。我发现上面的单行代码很难阅读,因为如果逐行扫描代码,很容易会漏掉一行的结尾处有一个return语句。


我将


validate重命名为validateNotSame

paths重命名为expectedPaths

以便更清晰。
/>
GraphFindAllPaths可能只是Graph

摘自Clean Code,第25页:


类和对象应具有名词或名词短语名称,例如CustomerWikiPage
AccountAddressParser。 [...]类名不应是动词。例如,考虑使用AllPathFinder而不是FindAllPaths


您可能想要在此处创建防御性副本:


public FindAllPaths(GraphFindAllPaths<T> graph) {
    if (graph == null) {
        throw new NullPointerException("The input graph cannot be null.");
    }
    this.graph = graph;
}



它会禁止恶意客户端修改类的内部数据,并可能为您节省一个几个小时的调试。 (有效的Java,第2版,第39项:在需要时进行防御性复制)

未使用或未测试Iterable功能和removeEdge方法。您也许可以删除它们。
测试方法通常在Test文件(例如,FindAllPathsTest)中,并由JUnit运行。它们通常位于单独的源文件夹中,该文件夹不使junit.jar(和其他测试依赖项)与生产代码一起打包。 Maven的目录结构是众所周知的项目结构,请考虑使用它(以及Maven,Gradle或任何其他构建工具)。


评论


\ $ \ begingroup \ $
我确实同意您的命名约定(例如Graph),并为单元测试提供了单独的文件,但是对我个人而言,将它们全部合并到一个文件中对我来说更方便。虽然我同意他们的观点
\ $ \ endgroup \ $
– JavaDeveloper
2014年3月29日在18:41

\ $ \ begingroup \ $
您可能想在此处创建一个防御性副本-我也同意这一点,但是创建图的深层副本并不简单。
\ $ \ endgroup \ $
– JavaDeveloper
2014年3月30日下午5:55

#3 楼

这确实是一个有趣的问题。首先,我想提到恕我直言,此任务没有多项式解决方案,因为它不是优化问题。所需的结果不是某物的最大/最小,而是所有可能路径的枚举。我认为所有可能的路径都可能导致n!完整图中的不同路径,其中n是节点数。因此,找到解决方案取决于输入的大小。如果输入上有大图,则使用此算法将花费大量时间和内存,并且可能无法完成。我不知道此解决方案的确切用法,但我想它是一种地图/导航应用程序。如果您不介意,我会标记一些有用的观点并分享我的考虑事项:


我看不到您在哪里使用double值,该值在每个边上都有。我想它将用于计算路径成本并返回按此条件排序的路径。如果是这样,考虑每次使用一些流行的算法来解决最短路径问题时,仅一次找到一小部分路径

如果输入中有稀疏图,则将导致每个节点使用Map在许多重物中,只具有一些价值。这将不必要地增加内存消耗。我不知道用例,将使用哪种类型的节点(T),但我建议使用从int到T的单个映射,并使用整数作为节点号。然后,您可以使用double [] []矩阵保存边缘值,并使用整数作为矩阵中的索引。示例:

0->“ A”
1->“ B”
2->“ C”



然后,将像这样显示边“ A” --4.2->“ C”:matrix [0] [2] = 4.2。同样,这将导致矩阵中许多未使用的单元。甚至更好的解决方案将是使用单个阵列/列表保存每个节点的邻居阵列/列表。实际上,您正在遍历邻居,因此不需要HashMap,只需持有一个键-值对列表就足够了。我的意思是:List<List<Double>> nodes代表图,而node.get(i)是另一个包含对的列表像(nodeNumber,edgeCost)。使用前面的示例:如果使用ArrayList,请考虑提供较小的初始容量,因为我认为默认值为16。

希望您能找到解决该问题的可行方法:)。

评论


\ $ \ begingroup \ $
我非常不同意你的第二点。在这种情况下,使用Map >是表示稀疏图的客观最佳方法。如果该图不是稀疏的,那么在内存方面当然最好使用矩阵表示。您的List >想法似乎是使用Maps的一种特殊方法,但缺点是它也不是稀疏的(实际上与矩阵表示形式完全相同)。与矩阵表示相比,使用地图的优势在于,获取出站边集不必是O(n)操作。
\ $ \ endgroup \ $
–阿蒙
2014年3月29日10:29



\ $ \ begingroup \ $
如果您使用LinkedList,则List >不会有空单元格,但不会使用自扩展ArrayList。我同意您的观点,即使用map将使O(1)访问边缘,但是正如我所见,该算法会遍历所有邻居,并且不会通过键访问它们。我对使用HashMap的担心是它的默认加载因子为0.75,而最好的情况下,您将仅使用分配的内存的0.75%。达到负载系数时,它将自行调整大小(可能加倍)。实际上,您对使用地图有个很好的了解。这里的生产力取决于Map和List的实现。
\ $ \ endgroup \ $
–egelev
2014年3月29日上午11:05

\ $ \ begingroup \ $
如果使用List >,则double属于哪个边的唯一编码是double在列表中的位置。无论是否存在这种边缘,都必须表示每个可能的边缘。
\ $ \ endgroup \ $
– user2357112支持Monica
2014年3月29日在11:36