Skip to content

KorotinEgor - #1

Open
KorotinEgor wants to merge 2 commits into
ValentinV95:mainfrom
KorotinEgor:main
Open

KorotinEgor#1
KorotinEgor wants to merge 2 commits into
ValentinV95:mainfrom
KorotinEgor:main

Conversation

@KorotinEgor

Copy link
Copy Markdown

No description provided.

@KorotinEgor

Copy link
Copy Markdown
Author

@IljarSmirnov @kermanglobe
Можно проверять, но пока что не считает операции(или что там надо считать)

@kermanglobe kermanglobe left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

если мне не изменяет память, то должны ещё быть:

  • упорядоченная таблица на массиве
  • логирование времени на выполнение операций с таблицами

Comment thread samples/main_tables.cpp
Comment on lines +54 to +57
} while (str == "");
T1.add(str, res);
T2.add(str, res);
T3.add(str, res);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

зачем добавлять переменную во все три таблицы сразу?

@KorotinEgor KorotinEgor Apr 30, 2024

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Даже не знаю, Волокитин сказал.
Про массив в тз ничего нет) и у меня не записано
Про время я и сам выше написал

Comment thread src/arithmetic.cpp Outdated
Comment on lines +119 to +120
}
else if ((s[i] >= 'A' && s[i + 3]<='Z') || (s[i] >= 'a' && s[i + 3]<='z')) { //Work with polynomial names

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

забыл сюда впихнуть проверку на выход за пределы строки

Comment on lines +35 to +42

TEST(OrderedTable, delete_is_correct) {
OrderedTable<std::string, Polynom> T;
Polynom p = Polynom("1+x");
T.add("pol1", p);
T.del("pol1");
ASSERT_ANY_THROW(T.find("pol1"));
} No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

можно накинуть тестов с разными случаями удаления (удаление листа, удаление вершины с одним и двумя потомками, удаление головы без потомков или с потомками)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно то можно, но, зная свой код, все эти случаи сводятся к удалению вершины у которой нет хотя бы одного из потомков

Comment thread include/OrderedTable.h Outdated
Comment on lines +130 to +131
}
T find(const K& key_) const {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

лучше возвращать указатель на nd->data (текущая реализация предполагает копирование T при каждом обращении к таблице)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Идея была в том, чтобы не дать пользователю таким образом изменять таблицу. Хотя, не отрицаю, возможно это и излишне

Comment thread src/HashTable.cpp
Comment on lines +34 to +39

void HashTable::add(const std::string& s, const Polynom& P) {
try {
find(s);
}
catch (std::exception& e) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

к чему блок try/catch, если можно было сделать через if/else?

@KorotinEgor KorotinEgor Apr 30, 2024

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Проблема в том, что find бросает исключение, если не нашëл такого ключа, если бы я возвращал не элемент, а указатель, можно было бы действительно обойтись if/else, но я так не делал; почему? Описано выше

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Буду рад, если предложишь вариант, каким образом, возвращая элемент, а не ссылку на него, избавиться от try/catch

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

если возвращать элемент, а не указатель, то никак :)

Comment thread include/HashTable.h
Comment on lines +5 to +6
class HashTable {
std::pair<short int, std::pair<std::string, Polynom>>* mas;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

неплохо бы добавить комментарии, какое значение short int за что отвечает

@IljarSmirnov IljarSmirnov left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ошибок не найдено.

@KorotinEgor

Copy link
Copy Markdown
Author

@ValentinV95 можно проверять

@ValentinV95

Copy link
Copy Markdown
Owner

Лабораторная принята

@ValentinV95 ValentinV95 added the good first issue Good for newcomers label May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

good first issue Good for newcomers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants