-
Notifications
You must be signed in to change notification settings - Fork 1
separate module search as pull request #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…lasses Route, SearchInfo. Refactoring functions to signature to this classes. Broke FindRoute and tests
…sses, broke FindRoute method and tests
…ad in Search.FindRoute
|
Коммит 15fad5c был отправлен после дедлайна, поэтому проверяться не будет |
| virtual std::vector <BasePoint> getBasePoints() = 0; | ||
| virtual std::vector <Infrastructure> getInfrastructurePoints() = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
всё ещё считаю довольно странным наличие двух разных функций для этих двух сущностей в одном месте
стоило тут реализовать абстрактную фабрику
и использование std::vector в интерфейсе - не самая лучшая идея
| #include <iostream> | ||
| #include <vector> | ||
| #include <unordered_map> | ||
| #include <unordered_set> | ||
| #include <set> | ||
| #include <optional> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
алфавитный порядок
| using std::vector; | ||
| using std::unordered_map; | ||
| using std::unordered_set; | ||
| using std::optional; | ||
| using std::set; | ||
| using std::pair; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
делать using в header-файле === делать "подлянку" будущим пользователям
| public: | ||
| Dijkstra() {} | ||
|
|
||
| void AddDirectedEdge(const T &from, const T& to, unsigned int weight); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
это явно не задача класса Дейкстра
задача класса Дейкстра - это найти маршрут, получив граф и стартовую вершину
| using std::set; | ||
| using std::pair; | ||
|
|
||
| template <class T> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vertex
| } | ||
| } | ||
|
|
||
| Search::Search(DataBase* base) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
base может быть nullptr
| if (!idPointMap.count(id)) { | ||
| return optRoute; | ||
| } | ||
| dijkstraSearcher.FindRoute(id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
непонятно, почему dijkstra жёстко зашит внутрь класса
либо нужно было "движок" передавать аргументом в метод, либо аргументом в конструктор (или через шаблонный параметр), либо решить этот вопрос через наследование, по-разному реализуя логику обхода (шаблонный метод)
| std::vector<Edge> GetEdges() { return edges; } | ||
| unsigned int Size() { return edges.size(); } | ||
| void Reverse() { std::reverse(edges.begin(), edges.end()); } | ||
| iterator begin() { return links.begin(); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
что такое links - осталось непонятным...
| std::vector <BasePoint> graf; | ||
| std::vector <Infrastructure> infr; | ||
| std::map <std::string, std::vector <Point*> > namePointsMap; | ||
| std::map <unsigned int, Point*> idPointMap; | ||
| Dijkstra <unsigned int> dijkstraSearcher; | ||
|
|
||
| void createMapPoints(std::vector <Point*> points); | ||
| void initMaps(); | ||
| void initDijkstra(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
почитайте про PIMPL - мы об этом рассказывали на лекциях...
| } | ||
|
|
||
| vector <Infrastructure> fillInfrPoints() { | ||
| Infrastructure i9(9), i10(10), i11(11), i12(12), i13(13), i102(102), i103(103); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
плохие названия переменных, ещё и на одной строчке...
This is branch with code of Search module
developer - Artyom Volkov