Phonebook #5
Phonebook #5
Conversation
Made getter and setter in Subscriber. Subscriber seems ready
This reverts commit e6ec0b0.
hw4/Phonebook/Phonebook/Subscriber.h
Outdated
| struct Subscriber | ||
| { | ||
| // ����������� - �������� ����������� - ���������� | ||
| explicit Subscriber(char const * name, char const * number); |
There was a problem hiding this comment.
А у этого конструктора два аргумента, ему explicit не нужен :)
Хотя вреда от него нет, так что, наверное, правильно было бы писать explicit всегда (как const), но обычно его в таких случаях почему-то не пишут.
| Subscriber::Subscriber(Subscriber const & s) | ||
| { | ||
| this->name = nullptr; | ||
| this->number = nullptr; |
There was a problem hiding this comment.
Чтобы три раза это не писать, можно инициализировать поля прямо в месте их объявления, в самой структуре. Или воспользоваться constructor chaining-ом,
Subscriber::Subscriber(Subscriber const & s)
: Subscriber()
{
setName(s.name);
setNumber(s.number);
}| { | ||
| (*destination)[i] = str[i]; | ||
| } | ||
| (*destination)[size] = '\0'; |
There was a problem hiding this comment.
Есть strcpy, которая делает как раз это :)
|
|
||
| Phonebook::Phonebook(Phonebook const & p) {} // ����������� ����������� | ||
|
|
||
| void Phonebook::operator=(Phonebook const & p) {} // �������� ������������ |
There was a problem hiding this comment.
Нее, это делается в .h-нике:
Phonebook(Phonebook const & p) = delete; // конструктор копирования
void operator=(Phonebook const & p) = delete; // оператор присваивания| Phonebook::Phonebook() | ||
| { | ||
| int const size = 100; | ||
| base = new Subscriber[size]; |
There was a problem hiding this comment.
Тут, мне кажется, можно было просто new Subscriber[100];, всё равно size нигде больше не используется
| } | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Тоже, есть strcmp, которая и так всё делает.
| if (!file) | ||
| { | ||
| printf("FILE WAS NOT FOUND\n!"); | ||
| } |
There was a problem hiding this comment.
И тут тоже бы return, а то там ещё ниже fclose, ну и вообще, лишний else
hw4/Phonebook/Phonebook/main.cpp
Outdated
|
|
||
| menu(); | ||
|
|
||
| system("pause"); |
hw4/Phonebook/Phonebook/main.cpp
Outdated
| p.addNote(s2); | ||
| p.addNote(s3); | ||
| p.findNumberByName("Tralala"); | ||
| printf("Test passed if written: Tralala 567\n"); |
There was a problem hiding this comment.
Ну лучше бы возвращать массив результатов и проверять автоматически. Тесты по идее должны выполняться при каждой сборке, и никто не будет специально сидеть и проверять, что всё ок.
hw4/Phonebook/Phonebook/main.cpp
Outdated
| printf("Tralala 567\n"); | ||
| printf("Lalala 900\n\"\n"); | ||
| p.saveToFile(); | ||
| printf("Check file \"phonebook\" to finish testing\n"); |
There was a problem hiding this comment.
И тут --- имеет смысл открыть файл, прочитать и убедиться, что всё ок
| const int readBytes = fscanf(file, "%s", buffer); | ||
| if (readBytes < 0) | ||
| { | ||
| break; |
There was a problem hiding this comment.
Так же утечка памяти будет? Ну да, буфер не заполнился, но память-то под него выделена, а тут мы теряем на него указатель.
| Subscriber* s = new Subscriber; | ||
| s->setName(data[i]); | ||
| s->setNumber(data[i + 1]); | ||
| (*this)[i / 2] = *s; |
There was a problem hiding this comment.
В this кладутся только значения Subscriber, а не указатель на неё? Тогда память, выделенная под структуру, потеряется. Можно было её не на куче, а на стеке вызовов прямо выделять.
| { | ||
| delete[] data[i]; | ||
| } | ||
| } |
There was a problem hiding this comment.
Мне кажется, что можно вообще все данные в data не считывать, а прямо прочитали две строки, разобрали их в Subscriber, запихали в базу и т.д. Но это просто наблюдение, можно это не править.
hw4/Phonebook/Phonebook/tests.cpp
Outdated
| // printf("Lalala 900\n\"\n"); | ||
| // p.saveToFile(); | ||
| // printf("Check file \"phonebook\" to finish testing\n\n\n"); | ||
| //} |
| { | ||
| for (int i = 0; i < size(); ++i) | ||
| { | ||
| if (strcmp((*this)[i].getNumber(), number)) |
There was a problem hiding this comment.
strcmp возвращает 0, если строки равны, что соответствует false, если привести его к bool-у. Поэтому неявные преобразования типов --- зло. Этот код находит все записи, кроме той, что нужна :)
| Subscriber s1("Ololo", "1234"); | ||
| Phonebook p; | ||
| p.addNote(s1); | ||
| p.saveToFile(); |
There was a problem hiding this comment.
После этого тест не удаляет добавленные данные, поэтому сколько раз мы запустили программу --- столько строк "Ololo 1234" будет в телефонной книге :)
Program phonebook does these things:
0 - exit
1 - add note (name and phone number)
2 - print all notes
3 - find number by name
4 - find name by number
5 - save current data