Draft: For comments only #3
Conversation
Create cmake.yml
Update cmake.yml
remove googletest from CMakeLists.txt
delaunay/DelaunayGraph.h
Outdated
| [[nodiscard]] bool operator<(Edge const &other) const; | ||
| }; | ||
|
|
||
| void add_terminal(GridUnit x_coord, GridUnit y_coord, NodeId terminal_id = -1); |
There was a problem hiding this comment.
Bei sowas wie NodeId terminal_id, sind Werte mit namen immer besser als einfach Zahlen.
Hier z.B. in der typedefs.h
NodeId constexpr invalid_node = -1
delaunay/DelaunayGraph.h
Outdated
|
|
||
| void add_edge(Terminal terminal_a, Terminal terminal_b); | ||
|
|
||
| void calculate(); |
There was a problem hiding this comment.
Hier gibt es calculate und die translate funktionen.
Insbesondere ist ein DelaunayGraph nicht notwendigerweise eine delaunay-triangulierung.
Man sollte immer versuchen Algorithmus und Datenstruktur zu trennen.
Dh. hier sollten Graph und delaunay-triangulierungs-algo getrennt werden.
Stattdessen eine Klasse Graph
und eine funktion Graph compute_delaunay_triangulation(std::vector<Terminal> const& terminals),
die nicht member einer Klasse sind
Eine Datenstruktur Kapselt die beinhalteten Daten und exportiert ein Wohldefiniertes interface, das dafuer sorgt, dass die internen Daten immer in einem Konsistenten Zustand sind (das ist das Ziel, geht nciht immer 100%)
Ein algorithmus macht operationen auf diesen Daten mithilfe des gegebenen interface.
Auf diese Weise koennen die einzelnen teile einfacher wieder verwendet werden und sind einfacher wartbar.
delaunay/DelaunayGraph.cpp
Outdated
| } | ||
|
|
||
| void DelaunayGraph::calculate() { | ||
| DelaunayPriorityQueue X(_max_x, _max_y); |
There was a problem hiding this comment.
Gib denen lieber hilfreiche namen. x_coordinate_queue oder so. Genauso unten mit Y.
delaunay/DelaunayGraph.cpp
Outdated
| int age_counter = 0; | ||
|
|
||
| while (not X.empty()) { | ||
| auto P = X.extract_min(); |
delaunay/DelaunayGraph.cpp
Outdated
| age_counter++; | ||
| if (P.status == ACTIVE) { | ||
| Y.insert(P.terminal); | ||
| auto successor = Y.successor(P.terminal); |
| #ifndef HAUPTAUFGABE_2_HEAP_H | ||
| #define HAUPTAUFGABE_2_HEAP_H | ||
|
|
||
|
|
There was a problem hiding this comment.
Heap concept und dann template functions, wo es verwendet wird waere deutlich schneller.
| bool operator<(Node const &other) const; | ||
| }; | ||
|
|
||
| public: |
kruskal/disjoint_set_2.h
Outdated
| @@ -0,0 +1,115 @@ | |||
| #pragma once | |||
kruskal/disjoint_set_2.h
Outdated
| #include <iostream> | ||
| #include <vector> | ||
|
|
||
| class Disjoint_Set { |
There was a problem hiding this comment.
Benennungsschema unterscheidet sich vom rest.
| return return_graph; | ||
| } | ||
|
|
||
| DelaunayGraph kruskal(DelaunayGraph const &delaunay_graph) { |
There was a problem hiding this comment.
Hier sieht man, was ich meinte mit einfacherer wiederverwendung.
2 mal die gleicht Funktion, nur einmal mit Graph und einmal mit DelaunayGraph.
Code duplication sollte immer vermieden werden. Angenommen man findet einen bug, oder muss etwas aendern, weil neue anforderungen da sind. Dann muesste man immer beide stellen aendern.
Das fuehrt sehr leicht zu fehlern
accidentally committed wrong changes in the commit before, removed asan and ubsan
…ination Implement delaunay mehlhorn combination
Dieser pull-request ist nur da, damit ich ein review starten kann.