Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Server friends #1

Open
wants to merge 85 commits into
base: server_friends
Choose a base branch
from
Open

Server friends #1

wants to merge 85 commits into from

Conversation

andrew-kireev
Copy link
Collaborator

@andrew-kireev andrew-kireev commented Nov 2, 2020

Uml-диаграмма Сервера Friends
image

@andrew-kireev andrew-kireev changed the title Service frends — работа с социальными отношениями между людьми. Service frends Nov 11, 2020
@andrew-kireev andrew-kireev changed the title Service frends Service friends Nov 11, 2020
@andrew-kireev andrew-kireev changed the base branch from main to server_friends November 11, 2020 10:19
@andrew-kireev andrew-kireev changed the title Service friends Server friends Nov 11, 2020
// Тест парсера Json

TEST(Test_parser, parse_test_1) {
tcp_network::ParseJson parser;
Copy link
Collaborator

Choose a reason for hiding this comment

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

у тебя класс ParseJsonимеет 2 статических методы, для вызова статических методов нет необходимости создавать экземпляр класса, так как их можно вызвать напрямую


class ParseJson {
public:
explicit ParseJson() = default;
Copy link
Collaborator

Choose a reason for hiding this comment

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

а можно было обойтись дефолтным конструктором?

Comment on lines +29 to +31
# - sudo wget https://github.com/Kitware/CMake/releases/download/v3.19.1/cmake-3.19.1.tar.gz
# - sudo tar -zxvf cmake-3.19.1.tar.gz && cd cmake-3.19.1
# - sudo ./bootstrap && sudo make && sudo make install

Choose a reason for hiding this comment

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

Из рабочей версии стоит удалить закомменченный код

Copy link
Collaborator Author

@andrew-kireev andrew-kireev Dec 28, 2020

Choose a reason for hiding this comment

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

из финальной версии весь закомменченный и отладочный код будет убран

else()
set(CMAKE_CXX_STANDARD 17)
endif()
set(CMAKE_CXX_FLAGS "-lpqxx -lpq")

Choose a reason for hiding this comment

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

Лучше подключать библиотеки не через флаги, а через target_link_libraries

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

исправлю на поиск через find_package

using namespace boost::asio;


class Client {

Choose a reason for hiding this comment

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

У клиента будет бесконечный цикл, неясно, как корректно завершить работу с клиентом

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

да, есть сейчас такой недочёт. исправлю



boost::asio::write(socket, boost::asio::buffer(add_request.c_str(), add_request.size()));
std::cout << "Запрос отправлен" << std::endl;

Choose a reason for hiding this comment

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

Здесь и далее стоило выводить отладочную информацию в логи


using boost::asio::ip::tcp;

const int max_length = 2024;

Choose a reason for hiding this comment

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

Почему именно такое значение?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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

}
}

void Session::send_response(std::string& respones) {

Choose a reason for hiding this comment

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

При каждой отправке сообщения создается новый клиент, возможно стоило не разрывать соединение

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

да, такая проблема есть. но необходимость поддерживать открытое соединение между всеми серверами в нашей сети сильно бы усложнило логику

#else
BOOST_LOG_TRIVIAL(error) << e.what();
#endif
return {0, 0};

Choose a reason for hiding this comment

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

Выглядит очень странно, непонятно, что означает {0, 0}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

показатель того, что к нам пришли несуществующие порт и ip

Comment on lines +36 to +44
auto iter = request.find('\n');
requsts.insert({"destination", request.substr(0, iter)});
request.erase(0, iter + 1);
iter = request.find('\n');
requsts.insert({"priority", request.substr(0, iter)});
request.erase(0, iter + 1);
iter = request.find('\n');
requsts.insert({"type", request.substr(0, iter)});
request.erase(0, iter + 2);

Choose a reason for hiding this comment

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

Возможно стоило вынести это в отдельные/отдельную функцию, что сделает логику работы понятнее

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

может быть, но эта функция и так отвечает за парсинг


#include "friends_data_base.h"

int main(int argc, char **argv) {

Choose a reason for hiding this comment

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

Не стоит проверять работоспособность БД прямо в main

@@ -0,0 +1,324 @@
//

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.

None yet

3 participants