test#1
Conversation
|
|
||
| #include <algorithm> | ||
| #include <array> | ||
| #include <cassert> |
There was a problem hiding this comment.
Я так и не понял, зачем тебе cassert, если ты не пользуешь что-то оттуда. Она нам понадобиться, но проверь все хедеры, вдруг ты что-то не пользуешь
| void RemoveRandomElementsFromMap(std::map<int, int>& random_map) { | ||
| auto indexes_to_delete = GenerateIndexesToDelete(random_map.size()); | ||
|
|
||
| std::vector <std::map<int, int>::iterator> iterators_to_delete; |
There was a problem hiding this comment.
Пробегись каким-нибудь clang-format, а то у тебя то std::vector<T>, то std::vector <T>
| size_t prev_index = 0; | ||
| auto it = random_map.begin(); | ||
| for (const auto& index : indexes_to_delete) { | ||
| size_t offset = index - prev_index; |
There was a problem hiding this comment.
Ты уверен что prev_index будет всегда меньше, чем index? Выглядит так, что тут возможно переполнение. Тесты бы это показали, кстати.
| const size_t MAX_COUNT_TO_DELETE = 15; | ||
|
|
||
| int GenerateRandomNumber(int minimum = MINIMUM, int maximum = MAXIMUM) { | ||
| static std::default_random_engine re(20200620); |
There was a problem hiding this comment.
https://en.cppreference.com/w/cpp/numeric/random
Вот тут в самом низу есть пример, как можно делать рандомизацию.
У тебя сейчас генераторы в качестве сида используют одно и то же число, уверен, что числа идут рандомные из этой функции?
| return is_in_collection; | ||
| } | ||
|
|
||
| std::array <bool, MAXIMUM + 1> GetElementExistingMask(const std::vector<int>& collection) { |
There was a problem hiding this comment.
Почему так, а не через маску?
Ну Т.е. ты мог бы ужать это до одного size_t и тупо возвращать один size_t.
А когда тебе надо прочитать/записать значение mask && (1 << idx) ну или mask || (1 << idx) (в случае записи)
Просто это неэффективно в твоём случае ни по памяти, ни по времени.
Не знаю, насколько критично тебе это менять, но вот статейка: https://howardhinnant.github.io/onvectorbool.html
А вот разница: https://stackoverflow.com/questions/44607389/behaviour-of-stdarraybool-vs-stdvectorbool
Т.е. твоя штука, если бы не было NRVO копировала бы MAXIMUM + 1 интов, а т.к. оно есть, то у тебя просто храниться MAXIMUM + 1 интов на стеке, и ванугю, что доставание их по индексу сильно медленнее, чем работа с маской. Хотя я могу быть тут не до конца прав. Погляди на обе ссылки, подумай стоит ли оставлять так.
| auto random_vector = GenerateRandomVector(); | ||
| auto random_map = GenerateRandomMap(); | ||
| RemoveRandomElementsFromVector(random_vector); | ||
| RemoveRandomElementsFromMap(random_map); | ||
| Synchronyze(random_vector, random_map); |
There was a problem hiding this comment.
Нет ни вывода никакого в консоль/ни тестов/ни рандомизированных тестов. Непонятно, как доказать работоспособность программы.
Советую тебе вытащить все эти функции отдельно, а тут написать что-то вроде:
int main() {
RunTests();
RunProgram();
}
RunTests - будут состоять из assert-ов простых (ну т.е. не стоит тащить никаких тестовых фреймворков) и в случае чего должны кидать исключении
Так же желательно в RunTests запихнуть один тест, который будет запускаться 100500 раз, чтобы убедиться, что ты нигде по памяти не проехали или не натворил чего плохого.
RunProgram - просто обычный запуск программы
1 participant
No description provided.