Conversation
Kapitonov
left a comment
Kapitonov
left a comment
There was a problem hiding this comment.
Если предполагается реальное использование библиотеки, то у вас довольно моного логгирования в коде. Есть вероятность, что большую часть времени ваше приложение будет им и заниматься.
|
|
||
| bool Connection::IsOpen() const { return socket_.isValid(); } | ||
|
|
||
| void Connection::LogErrorAndThrow() { |
There was a problem hiding this comment.
Название функции не отображает её сути.
There was a problem hiding this comment.
Почему?
она же как раз и делает то, что написанно?
|
|
||
| void Connection::LogErrorAndThrow() { | ||
|
|
||
| socket_ = desc::Descriptor{}; |
There was a problem hiding this comment.
А socket_.close() не тоже самое сделает?
| } | ||
|
|
||
| void Server::LogErrorAndThrow() { | ||
| server_socket_ = desc::Descriptor{}; |
There was a problem hiding this comment.
А sercver_socket_.close() не тоже самое сделает?
| void Connect(const std::string &addr, int port); | ||
| }; | ||
|
|
||
| class Server { |
There was a problem hiding this comment.
Реализация в разных файлах, давайте и объявления разнесем.
| client_fd = desc::Descriptor{::accept( | ||
| *server_socket_, reinterpret_cast<sockaddr *>(&client_sock_addr), &s)}; | ||
| } catch (const desc::DescriptorError &err) { | ||
| LogErrorAndThrow(); |
There was a problem hiding this comment.
А в этом месте точно надо закрывать соккет сервера?
Так это все Debug уровень логирования, мы при инициализации программы можем установить уровень логгера в warn или Info и будет куда меньше логов, и вообще, я считаю, что логи это очень хорошо, если я буду пользоваться библиотекой, которая может полностью отлогировать себя при моем желании я буду очень рад) |
Kapitonov
left a comment
Kapitonov
left a comment
There was a problem hiding this comment.
Можно сливать после исправления.
|
|
||
| void Close(); | ||
|
|
||
| size_t Read(char *, size_t); |
There was a problem hiding this comment.
В других функциях вы пишете названия переменных.
| ssize_t bytes_read = ::read(*socket_, data, len); | ||
|
|
||
| if (bytes_read < 0) { | ||
| LogErrorAndThrow(); |
There was a problem hiding this comment.
Лучше кинуть исключение.
| ssize_t bytes_wrote = ::write(*socket_, data, len); | ||
|
|
||
| if (bytes_wrote < 0) { | ||
| LogErrorAndThrow(); |
2 participants
Тестировал через net cat