Task 10 by YaroslavKomarov · Pull Request #2 · YaroslavKomarov/HomeTasksCPP

YaroslavKomarov · 2020-12-12T07:55:39Z

No description provided.

Fixed the .gitignore file.

Deleted .sln file; Deleted .vcxproj.filters file.

Improved the hasConnection method in Maze.cpp; A program has been written to display a new maze and tree.

Toxin65 · 2020-12-22T10:30:33Z

Определитесь, пожалуйста, что из этого мне смотреть.
Заявлено было 8,9. Вижу два ПР, и только в одном что-то про Таск 10...

YaroslavKomarov · 2020-12-22T15:07:23Z

Я, честно говоря, затрудняюсь ответить. Я проходил ревью у Елены Александровны и она мне сказала, внести последнюю правку, после чего она могла бы принять у меня task 8 и task 9. Однако к тому моменту я уже выполнили и task 10, который по просьбе Елены Александровны поместил в отдельный pull

Toxin65 · 2020-12-22T16:14:34Z

Ну, я при просмотре 10го накидаю замечаний по всем трём, чует моё сердце...
Так что может разницы то и нет.
Тем не менее, мне нужны чёткие указания. Что мне смотреть и в какую графу потом писать, соответственно.

YaroslavKomarov · 2020-12-23T12:21:35Z

Елена Александровна мне сказала, что Вы будете проверять у меня 10 задание.

YaroslavKomarov · 2020-12-23T12:27:16Z

Также Елена Александровна сказала, что сможет зачесть мне 8 и 9 задание, если Вы зачтете мне 10)

Toxin65 · 2020-12-25T14:05:19Z

Ок, гляжу.

Toxin65

Исправьте то, что есть. Потом продолжим.
Извините, немного недочитал, надо уделить внимание всем, по возможности...

Practise_18.11.20/Practise_18.11.20/MCell.h

+#pragma once
+
+class MCell {
+private:


Practise_18.11.20/Practise_18.11.20/MCell.cpp

+
+MCell::MCell() {
+	m_down = false;
+	m_right = false;


Practise_18.11.20/Practise_18.11.20/MCell.cpp

+	m_right = false;
+}
+
+bool MCell::down() const { return m_down; }


Practise_18.11.20/Practise_18.11.20/MTreeNode.h

@@ -0,0 +1,26 @@
+#pragma once
+#include <vector>
+using namespace std;


Practise_18.11.20/Practise_18.11.20/MTreeNode.h

+private:
+	int m_i, m_j;
+	int m_distance;
+	MTreeNode* m_parent;


Toxin65 · 2020-12-25T14:45:47Z

Practise_18.11.20/Practise_18.11.20/Practise_18.11.20.cpp

+void PrintTreeInfo(const MTreeNode* tree, const int& width, const int& height);
+vector<int> GetRandCoordinates(const int& width, const int& height);
+
+void PrintTreeInfo(const MTreeNode* tree, const int& width, const int& height) {


Ох, как плохо без комментариев.
Что делает эта функция? Зачем программист её написал?

Функция для получения произвольных координат на границе рандомного лабиринта. Теперь этот комментарий прописал в коде

Toxin65 · 2020-12-25T14:46:02Z

Practise_18.11.20/Practise_18.11.20/Practise_18.11.20.cpp

+	vector<vector<string>> to_print(height);
+	for (auto& vctr : to_print)
+		vctr = vector<string>(width);
+	while (true) {


Toxin65 · 2020-12-25T14:46:33Z

Practise_18.11.20/Practise_18.11.20/Practise_18.11.20.cpp

+		int i = tree->i();
+		int j = tree->j();
+		sum_weights += tree->distance();
+		max_weight = tree->distance() > max_weight ? tree->distance() : max_weight;


Toxin65 · 2020-12-25T14:48:20Z

Practise_18.11.20/Practise_18.11.20/Practise_18.11.20.cpp

+	}
+	for (auto& vctr : to_print) {
+		for (auto& str : vctr)
+			cout << setw(5) << str;


Неплохо...

Тоже забыл это поправить перед отправкой! В setw() теперь передаю длину строки, полученной от вершины с максимальным весом + 1 (const int max_len = size(to_string(max_weight) ....setw(max_len) << str...)

Toxin65 · 2020-12-25T14:48:35Z

Practise_18.11.20/Practise_18.11.20/Practise_18.11.20.cpp

+			cout << setw(5) << str;
+		cout << endl;
+	}
+	int average_weight = sum_weights / (1. * width * height);


Однострочные методы инициализированны в заголовочных файлах; Убрал из заголовочников

Одностроные методы проинициализированны в заголовочных файлах; В заголовочных больше не используются пространства имен; const есть везде, где можно; Исправлен нелепый метод searchNode; Добавлен метод поиска корня в дереве searchRoot; во всех заголовочниках сначала идет public-секция; При живом родителе у дочернего элемента distance правильный; Лишние ссылки при прохождении по векторам (и при int) убраны; Перед добавлением ребенка происходит проверка на его наличие к родителя; m_dict из класса Maze больще не член класса и он sctatic; добавлены assert при попытке получения клетки лабиринта и где-то еще; Добавлена проверка на соседство двух клеток для методов класса Maze; Теперь, если звязи не было метод removeConnection возвращает корректное значение; Вообще, кажется, все исправил, что было прописано Вами.

Toxin65

Давайте поправим...
Всё бы хорошо. Но есть непростительное!

Toxin65 · 2020-12-26T18:57:44Z

Practise_18.11.20/Practise_18.11.20/MCell.cpp

@@ -0,0 +1,5 @@
+//#include "MCell.h"


Это можно было бы и остаивть. Тогда он откомпилится в отдельный объектный файл. А это неплохо!

Toxin65 · 2020-12-26T18:58:41Z

Practise_18.11.20/Practise_18.11.20/MTreeNode.h

+	const MTreeNode* searchNode(const int i, const int j);
+	const MTreeNode* searchRoot(const MTreeNode* node);
+private:
+	int m_i, m_j = 0;


Что-то тут копипастой запахло.
Где-то уже видел...

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

Дык не про то речь... Ну и ладно...

Toxin65 · 2020-12-26T19:01:06Z

Practise_18.11.20/Practise_18.11.20/MTreeNode.cpp

+
+MTreeNode::MTreeNode(MTreeNode* parent) {
+	m_parent = parent;
+	m_distance = parent == nullptr ? 0 : parent->m_distance + 1;


Можо не тернарным, а просто при наличии родителя у него спросить и +1...

Practise_18.11.20/Practise_18.11.20/MTreeNode.cpp

+		if (child->m_i == i && child->m_j == j)
+			return false;
+	m_children.push_back(new MTreeNode(this));
+	m_children[m_children.size() - 1]->m_distance = this->m_distance + 1;


Toxin65 · 2020-12-26T19:03:42Z

Practise_18.11.20/Practise_18.11.20/MTreeNode.cpp

+}
+
+MTreeNode* MTreeNode::hasChild(int i, int j) const{
+	const int count = m_children.size();


Это тут зачем. Осталось?

Toxin65 · 2020-12-26T19:14:35Z

Practise_18.11.20/Practise_18.11.20/Maze.cpp

+bool Maze::hasConnection(int i1, int j1, int i2, int j2) const {
+	if ((abs(i1 - i2) + abs(j1 - j2) != 1)) return false;
+	if (j2 < 0 || j2 >= m_width || i2 < 0 || i2 >= m_height)
+		return false;


Может вынести эти штучки отдельно. А то дублирование получается. Ниже то же самое написано...

Toxin65 · 2020-12-26T19:15:39Z

Practise_18.11.20/Practise_18.11.20/Maze.cpp

+		return m_field[ind1 * m_width + ind2].down();
+	}
+	else
+		return false;


Тут вообще ассерт, такого быть то не должно в природе!
Ассерт -- хорошая проверка программиста, что он что-то накосячил...

Toxin65 · 2020-12-26T19:26:26Z

Practise_18.11.20/Practise_18.11.20/Maze.cpp

+	if (j2 < 0 || j2 >= m_width || i2 < 0 || i2 >= m_height)
+		return false;
+	int ind1 = min(i1, i2);
+	int ind2 = min(j1, j2);


const им всем же!

Toxin65 · 2020-12-26T19:27:24Z

Practise_18.11.20/Practise_18.11.20/Maze.cpp

+		if (j + 1 == m_width)
+			cout << endl;
+	}
+}


Пустая строка же!!!

Toxin65 · 2020-12-26T19:28:51Z

Practise_18.11.20/Practise_18.11.20/Practise_18.11.20.cpp

+	int sum_weights = 0;
+	int max_weight = 0;
+	queue<const MTreeNode*> nodes;
+	vector<vector<string>> to_print(height);


Да пусть будет...
Но предупреждение вы получили.
При приёме на работу я такую фигню не потерплю!!!

в конструкторе MTreeNode упразднил тернарный оператор; заменил индексацию для вектора в методе addChild на .back; убрал вычисление m_distance из addChild; убрал лишнее вычисление длинные вектора детей в hasChild; Вернул ссылку для итерирования по вектору в hasChild; Прописал несколько комментариев для serchNode, searchRoot; Убрал return nullptr у метода searchNode; Уменшил вложенность для большей читаемости кода у searchRoot; Исправил assert у метода cell класса Maze; Вынес в отделный метод checkConnection проверку на вшивость для всех связей; Добавил в метод checkConnecion проверку нереального случая; добавли нужные const; довил пустую строку в конце Maze.cpp.

Toxin65

Беды с памятью... Большие беды! Разберитесь с new delete..

Toxin65 · 2020-12-26T20:52:05Z

Practise_18.11.20/Practise_18.11.20/MTreeNode.cpp

+	return nullptr;
+}
+
+const MTreeNode* MTreeNode::searchNode(const int i, const int j) {


Доки надо в заголовках. Ну да ладно... На будущее...
Ну и кодировка похерилась...

Practise_18.11.20/Practise_18.11.20/MTreeNode.cpp

+#include <iostream>
+#include <queue>
+#include "MTreeNode.h"
+using namespace std;


YaroslavKomarov · 2020-12-26T22:12:49Z

Practise_18.11.20/Practise_18.11.20/MTreeNode.cpp

+
+MTreeNode::~MTreeNode() {
+	for (auto ptr : m_children)
+		delete ptr;


Речь о том, что я мог просто вызвать delete[] ? Или я оставляю в памяти висеть детей того родителя, у которого вызывается деструктор?

Сорян, всё норм. Я уже обревьювился в конец...

YaroslavKomarov · 2020-12-26T22:14:51Z

Practise_18.11.20/Practise_18.11.20/MTreeNode.cpp

+using namespace std;
+
+MTreeNode* MTreeNode::begintTree(int i, int j) {
+	MTreeNode* node = new MTreeNode(nullptr);


Тут утечка, верно? Кстати говоря, я вызывал delete для корня дерева в practise_18.11.20.cpp в случае построения второго лабиринта (про третий благополучно забыл). Но я догадываюсь, что это не есть хорошо. Можете намекнуть на пример корректного поведения для beginTree?

Извиняюсь, запутался... Всё норм.

Toxin65

Не идеально... Но зачесть можно.

Toxin65 · 2020-12-27T13:24:42Z

Practise_18.11.20/Practise_18.11.20/MTreeNode.h

+	const MTreeNode* child(int i) const { return m_children[i]; }
+	static MTreeNode* begintTree(int i, int j);
+	const MTreeNode* searchNode(const int i, const int j);
+	const MTreeNode* searchRoot(const MTreeNode* node);


Оба метода сами по себе const.

Toxin65 · 2020-12-27T13:31:30Z

Practise_18.11.20/Practise_18.11.20/MTreeNode.cpp

+#include "MTreeNode.h"
+using namespace std;
+
+MTreeNode* MTreeNode::begintTree(int i, int j) {


Ну ё-маё... А я его ищу. beginTree!!! А не begint...

Toxin65 · 2020-12-27T13:34:40Z

Practise_18.11.20/Practise_18.11.20/MTreeNode.cpp

+
+MTreeNode::~MTreeNode() {
+	for (auto ptr : m_children)
+		delete ptr;


Сорян, всё норм. Я уже обревьювился в конец...

Toxin65 · 2020-12-27T13:36:56Z

Practise_18.11.20/Practise_18.11.20/MTreeNode.cpp

+}
+
+MTreeNode* MTreeNode::hasChild(int i, int j) const{
+	for (auto &child : m_children){


Нет смысла ссылку делать. Получается ссылка на указатель -- это тут лишнее...

Toxin65 · 2020-12-27T13:38:36Z

Practise_18.11.20/Practise_18.11.20/Practise_18.11.20.cpp

+int main()
+{
+	const int maze_size = 5;
+	Maze maze1 = Maze(maze_size, maze_size);


Maze maze1(maze_size, maze_size);

Конечно же исправил beginTree))); убрал ссылку в hasChild; Теперь корректно создаю экземпляры класса Maze.

YaroslavKomarov added 9 commits November 27, 2020 17:22

Added .gitignore.

c3058b9

practise start

ab9df4e

Removed additional methods;

cf76b89

Fixed the .gitignore file.

Deleted .vcxproj file;

dced4f1

Deleted .sln file; Deleted .vcxproj.filters file.

Removed the additional GetChar method

ab80e5f

Added class MTreeNode;

6a71180

Improved the hasConnection method in Maze.cpp; A program has been written to display a new maze and tree.

В заголовочных вайлах теперь только объявления пременных и функций.

5a49c52

Новый метод searchNode добавлен в класс MTreeNode.

26cb0ff

В Practise_18.11.20.cpp добавлены методы для реализации task 10.

a442304

Toxin65 suggested changes Dec 25, 2020

View reviewed changes

YaroslavKomarov added 3 commits December 26, 2020 13:18

<F12#

b3a3245

Все поля проинициализированны при объявлении;

3bd31b1

Однострочные методы инициализированны в заголовочных файлах; Убрал из заголовочников

Toxin65 suggested changes Dec 26, 2020

View reviewed changes

YaroslavKomarov added 2 commits December 27, 2020 01:27

Добавил пустую строку в конце каждого файла.

df3698f

Toxin65 reviewed Dec 26, 2020

View reviewed changes

Toxin65 approved these changes Dec 27, 2020

View reviewed changes

Добавил const к методам searchNode, searchRoot;

ec25868

Конечно же исправил beginTree))); убрал ссылку в hasChild; Теперь корректно создаю экземпляры класса Maze.

Conversation

YaroslavKomarov commented Dec 12, 2020

Uh oh!

Toxin65 commented Dec 22, 2020

Uh oh!

YaroslavKomarov commented Dec 22, 2020

Uh oh!

Toxin65 commented Dec 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

YaroslavKomarov commented Dec 23, 2020

Uh oh!

YaroslavKomarov commented Dec 23, 2020

Uh oh!

Toxin65 commented Dec 25, 2020

Uh oh!

Toxin65 left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Toxin65 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Toxin65 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Toxin65 commented Dec 22, 2020 •

edited

Loading

Toxin65 left a comment •

edited

Loading