Skip to content

Neytrinoo exam 7 - #11

Open
a-badin wants to merge 9 commits into
neytrinoo_exam_4from
neytrinoo_exam_7
Open

a-badin wants to merge 9 commits into
neytrinoo_exam_4from
neytrinoo_exam_7

Conversation

@a-badin

@a-badin a-badin commented Dec 16, 2021

Copy link
Copy Markdown
Collaborator

No description provided.

@a-badin a-badin left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Исправляйте замечания и коммититьте в main

/*
* Структура с колбеками, которые будут вызываться после успешного чтения и записи соответственно.
*/
struct ServerConnectionCallbacks {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Обычно вместо этого делают интерфейс
struct IServerListener {
virtual onReadCb(std::string &&) = 0;
virtual onWriteCb(boost::system::error_code &err) = 0;
};


class ServerConnection {
public:
ServerConnection(std::string &&url, int port, ServerConnectionCallbacks &&callbacks);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

не увлекайтесь rvalue, копирование 20 байт быстрая операция, а вот передать константную строку вы уже не сможете


void read();

void readHandler(boost::system::error_code &err, size_t bytes_transferred);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

по-моему в колбеке const

#include <condition_variable>
#include <atomic>

#define BUFFER_LENGTH 1024

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

constexpr int BUFFER_LENGTH = 1024; забудьте про макросы и define

#include <atomic>

#define BUFFER_LENGTH 1024
#define END_STR "\r\n"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

const std::string или string_view

std::cerr << "delete ServerConnection" << std::endl;
(*pos)->stop();
connections_.erase(pos);
} catch (...) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

нехорошо игнорировать ошибки

}

SharedDocumentServer::~SharedDocumentServer() {
delete documentCommandBus_;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

unqiue_ptr, удалять руками очень опасно

IManageCommand::IManageCommand(command_t type_command) {
switch (type_command) {
case GET_DOCUMENT:
letter_ = new GetDocument();

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

new delete плохая практика, unique_ptr

return false;
}

std::shared_ptr<SharedDocumentServer> shared_document(

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

auto shared_document = std::make_shared(...), экономит 1 алокацию памяти


SharedDocumentServer::SharedDocumentServer(boost::asio::io_context &service) : server_(start_since_port++, service,
ServerCallbacks{
[this](std::shared_ptr<Connection> connection) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

заведите интерфейс

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants