-
Notifications
You must be signed in to change notification settings - Fork 26
Киракосян, Орлова, багтрекер #36
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
base: master
Are you sure you want to change the base?
Conversation
| /* Now connect to the server */ | ||
| if (connect(sockfd, (struct sockaddr *) &serv_addr, sizeof(serv_addr)) < 0) { | ||
| std::cerr << "ERROR connecting\n"; | ||
| exit(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Желательно, конечно, не делать exit прямо посреди I/O логики, завершением программы должен управлять отдельный слой кода.
| std::string client::getString() { | ||
| int length = getNum(); | ||
| char* buf = new char[length + 1]; | ||
| int n = read(sockfd, buf, length); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| char* buf = new char[length + 1]; | ||
| int n = read(sockfd, buf, length); | ||
| buf[length] = '\0'; | ||
| std::string ans(buf); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Если вы знаете размер посылки, то можно сразу создать std::string ans нужной длины и передавать and.data() в read.
| std::cout << "You not authorize\n"; | ||
| break; | ||
| } | ||
| case CHECK_RIGHTS: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Слишком много похожего кода. т.е. дублирование
| uint32_t bugId, verificationCode; | ||
| readInt32(sock_fd, bugId); | ||
| readInt32(sock_fd, verificationCode); | ||
| if (!client.isAuthorized()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Авторизация проверяется в нескольких функциях, поэтому, чтобы убрать дублирование, можно было бы вынести её куда-нибудь в process_client, чтобы проверять только из одной функции.
| _bugs_mutex.lock(); | ||
| if (_bugs.find(bugId) == _bugs.end()) { | ||
| _bugs_mutex.unlock(); | ||
| std::cout << "Developer with id " << client.user.id << " tried to fix non existing bug with id " << bugId << "\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Обратите внимание, что у вас очень часто встречается комбинания "вывели строку на экран, отправили число, отправили ещё число". Вполне кандидан на выделение в отдельную функцию.
| } else if (_bugs[bugId].status == Bug::BugStatus::QA){ | ||
| if (verificationCode == 0) { | ||
| _bugs[bugId].status = Bug::BugStatus::NEW; | ||
| _bugs_mutex.unlock(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Можно использовать lock guard, чтобы чуточку упростить код и повысить его надежность.
| } | ||
| char buf[length + 1]; | ||
| buf[length] = '\0'; | ||
| ssize_t n = read(sock_fd, buf, length); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
См. замечание к std::string client::getString().
No description provided.