UsersServer #10
UsersServer #10
Conversation
| project/includes/Server.h project/src/UsersDatabase.cpp) | ||
|
|
||
|
|
||
| target_link_directories(ServerUsers PUBLIC project/includes/) |
There was a problem hiding this comment.
у тебя же переменная для INC_DIR может стоит ее использовать? или зачем ты ее вводил?
| } | ||
| } | ||
|
|
||
| void server(boost::asio::io_service& io_service, unsigned short port) |
There was a problem hiding this comment.
зачем создавать отдельно io_service и передавать в эту функцию в качестве параметра?
| using std::string; | ||
| using boost::property_tree::ptree; | ||
|
|
||
| std::map<string, string> HandlerUser::parser_json(std::stringstream& request) { |
There was a problem hiding this comment.
нет смысла принимать stringstream в качестве аргумента, ведь у тебя по сути везде используются строки
| if (!request.str().empty()) { | ||
| boost::property_tree::read_json(request, pt); | ||
| std::map<string, string> result; | ||
| ptree::const_iterator end = pt.end(); |
There was a problem hiding this comment.
зачем создавать эту переменную отдельно? может стоит pt.end() использовать в цикле?
ServerUsers/main.cpp
Outdated
| int insert_user(std::map<string, string>& users_data) { | ||
| string sql_request("INSERT INTO Users VALUES("); | ||
| int i = 0; | ||
| for(auto& string : users_data) { |
There was a problem hiding this comment.
string не стоит использовать ключевые слова языка для цикла
| boost::asio::io_service& io_service_; | ||
| boost::asio::ip::tcp::socket socket_; | ||
| int max_length = 1024; | ||
| char data_[1024]; |
There was a problem hiding this comment.
зачем в каждом экземпляре инициализировать переменную int max_length ?
лучше сделать глобальную static переменную
если ты вводишь переменную для размера массива, то стоит использовать для инициализации размера массива
| #include <string> | ||
| #include <map> | ||
| #include <pqxx/pqxx> | ||
| #include "Database.h" |
| ~UsersDatabase() override; | ||
| int insert_(const std::map<std::string, std::string>& users_data) override; | ||
| const std::string data_user(int id); | ||
| const std::string all_users(); |
There was a problem hiding this comment.
какой смысл возвращать из функции 'const string', разумнее обозначать методы, которые не меняют не меняют состояние объект `std::string all_users() const'
| void Session::send_answer(std::string& answer) { | ||
| std::cout << "ООТвет " << answer << std::endl; | ||
| boost::asio::io_service service; | ||
| tcp::endpoint end(boost::asio::ip::address::from_string("127.0.0.1"), 8888); |
There was a problem hiding this comment.
у тебя создается io_service для каждого соединения что не очень хорошо
There was a problem hiding this comment.
надо вынести в параметры или конфиг
|
|
||
| W.exec( sql_request ); | ||
| W.commit(); | ||
| return 200; |
There was a problem hiding this comment.
нужна обертка для работы с БД
|
|
| project(ServerUsers) | ||
|
|
||
| set(CMAKE_CXX_STANDARD 17) | ||
| set(CMAKE_CXX_FLAGS "-lpqxx -lpq") |
There was a problem hiding this comment.
Запуск без флагов -Wall -Werror -Wpedantic
There was a problem hiding this comment.
поняла, спасибо!
|
|
||
| set(TESTS_DIR ${DIR}tests) | ||
|
|
||
| include_directories(project/includes/) |
There was a problem hiding this comment.
Лучше использовать target_include_directories
|
|
||
| include_directories(project/includes/) | ||
|
|
||
| add_library(ServerUsers STATIC ${SRC_DIR}/Session.cpp ${SRC_DIR}/HandlerUser.cpp |
There was a problem hiding this comment.
лучше создавать библиотеку внутри директории с исходниками и делать add_subdirectory
There was a problem hiding this comment.
запомню на будущее
|
|
||
| add_executable(test_server test_server.cpp) | ||
|
|
||
| add_executable(TESTS project/tests.cpp) |
There was a problem hiding this comment.
тесты должны быть в отдельной директории со своим симейком
| tcp::endpoint end(boost::asio::ip::address::from_string("127.0.0.1"), 8082); | ||
| tcp::socket socket(service); | ||
| socket.async_connect(end, [&socket] (const boost::system::error_code& err) { | ||
| // std::string requst = "123\napi/users/create/\n-1\n\n{\n" |
There was a problem hiding this comment.
Убрать закомменченный код, ему не место на репозитории
| std::map<string, string> result; | ||
| for (ptree::const_iterator it = pt.begin(); it != pt.end(); ++it) { | ||
| string field = std::string(it->first); | ||
| if (std::string(it->first) == "id") { |
There was a problem hiding this comment.
почему нет сущности, хранящей названия полей?
| request += "\n \"password\": \"" + data_user.at("password") + "\",\n \"login\": \"" + data_user.at("Eemail") + "\"\n}"; | ||
|
|
||
| boost::asio::io_service service; | ||
| boost::asio::ip::tcp::endpoint end(boost::asio::ip::address::from_string("127.0.0.1"), 9999); |
| request += std::to_string(data_base_.get_id()); | ||
| request += "\n \"password\": \"" + data_user.at("password") + "\",\n \"login\": \"" + data_user.at("Eemail") + "\"\n}"; | ||
|
|
||
| boost::asio::io_service service; |
There was a problem hiding this comment.
постоянно создается клиент, не поддерживается постоянное соединение
There was a problem hiding this comment.
чтобы это проблему исправить, пришлось бы менять архитектуру, а на это уже не было времени
| try { | ||
| data_base_.all_users(); | ||
| string result = std::to_string(number_request) + "\n"; | ||
| string new_result = data_base_.all_users(); |
| data_base_.all_users(); | ||
| string result = std::to_string(number_request) + "\n"; | ||
| string new_result = data_base_.all_users(); | ||
| if (new_result == "No users") { |
There was a problem hiding this comment.
Можно было бы сделать сущность, хранящую ответы от БД
No description provided.