Зубаков Алексей. Задание 1 #2
Conversation
|
Не собирается |
h31
left a comment
h31
left a comment
There was a problem hiding this comment.
Ещё не хватает описания протокола.
|
|
||
| using namespace operations; | ||
|
|
||
| struct Server { |
There was a problem hiding this comment.
Почему struct, если по факту это класс?
There was a problem hiding this comment.
Потому что мне лень писать public модификатор. По-моему, это вкусовщина.
There was a problem hiding this comment.
Даже стало интересно, а чем плохо писать struct?
There was a problem hiding this comment.
Семантическое значение. struct - это когда первичны данные, и есть некоторое количество кода для работы с ними (пример: точка с двумя координатами). class - когда первичен код, и есть некоторое количество полей для его работы.
Это был вопрос, а не замечание. Мне было вполне достаточно объяснения. Вкусовщина - да, поэтому и не замечание.
| std::shared_ptr<Operation> operation(parseRequest(socket)); | ||
| auto callback = [&]() { | ||
| int32_t result = operation.get()->evaluate(); | ||
| server.get()->sendResponse(socket, operation.get()->getType(), result); |
There was a problem hiding this comment.
Если перенести метод в класс RequestHandler, то не понадобится передавать ссылку на server.
There was a problem hiding this comment.
Мб, но кажется, что это не очень существенно. С вашего позволения оставлю как есть
| }; | ||
| std::async(std::launch::async, callback); | ||
| } catch (const std::exception &e) { | ||
|
|
| void execute() { | ||
| int clientSocket; | ||
| struct sockaddr_in address; | ||
| int opt = 1; |
There was a problem hiding this comment.
Эта переменная где-либо используется?
| operations::Operation *parseRequest(const int socket) { | ||
| std::unique_ptr<char[]> buffer(new char[REQUEST_SIZE]); | ||
|
|
||
| const ssize_t &ssize = read(socket, buffer.get(), REQUEST_SIZE); |
There was a problem hiding this comment.
Что произойдет, если read считает не всё сообщение целиком, а только часть?
| struct Operation { | ||
| Operation(Type type) : type(type) {} | ||
|
|
||
| virtual int32_t evaluate() const; |
There was a problem hiding this comment.
Почему именно int32_t? Объясните свой выбор.
There was a problem hiding this comment.
Во избежание вопросов про гарантии на то, что ос/компилятор, на которых будут запущены/скомпилированы клиент и сервер, обязательно используют одинаковый floating point encoding, например, IEEE-754. Насколько мне известно, в стандарте c++ это не специфицировано. Поэтому пересылка float -- такая себе затея, если нет какого-то конкретного протокола сериализации, сочинять велосипед не хочу.
В задании также не говорилось о том, что вычисления должны быть не целочисленными, поэтому никаких проблем не вижу.
There was a problem hiding this comment.
Если вопрос про почему не int, а int32_t, то ответ, по-моему, очевиден: не на всех платформах int 32-битный
There was a problem hiding this comment.
Тут скорее вопрос про то, почему не int64_t. Аргумент по поводу float и способов кодирования абсолютно уместный, согласен с ним.
| case Div: | ||
| return new operations::Division(arg1, arg2); | ||
| default: | ||
| perror(""); |
There was a problem hiding this comment.
Есть смысл уведомить клиента, если произошла какая-то ошибка (например, пришли некорректные данные).
| cmake_minimum_required(VERSION 3.12) | ||
| project(networks-labs) | ||
|
|
||
| set(TBB_DIR lib/tbb) |
There was a problem hiding this comment.
Где именно в вашем проекте используется TBB?
|
Принято. Оценка 9. Жду выполнения части, которая касается клиента (дедлайн уже давно прошел). |
2 participants
No description provided.