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

first commit #1

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

first commit #1

wants to merge 2 commits into from

Conversation

DivineSilverWolf
Copy link
Owner

No description provided.

Copy link

@dmazhuga dmazhuga left a comment

Choose a reason for hiding this comment

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

Ещё я на трансляции видел pom.xml, добавь его тоже в репозиторий - так гораздо удобнее скачивать и запускать твой проект.

while (true) {

message = in.nextLine();
byte[] dataSend = ByteBuffer.allocate(message.length() + MAKING_UP_SPACE_FOR_ACK_SEQ).put(message.getBytes(StandardCharsets.UTF_8)).array();

Choose a reason for hiding this comment

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

message.length() выдает длину в символах юникода, а ByteBuffer.allocate принимает аргументом размер в байтах. Из за этого вылетит ошибка переполнения буфера в тех случаях, когда в строке встречаются символы, которые в UTF-8 кодируются не одним байтом (например, кириллица).

Нужно вынести message.getBytes(StandardCharsets.UTF_8) в отдельную переменную, затем вместо message.length() использовать длину этого массива.

private static final String ACK_EQUALS = ", ACK=";
public static void main(String[] args) throws IOException {
InetAddress inetAddress = InetAddress.getLocalHost();
DatagramSocket client = new DatagramSocket();

Choose a reason for hiding this comment

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

Сокет нужно закрывать, по-хорошему в try-with-resources.


public class Client {
private static final byte STARTER=1;
private static final int SET_SEQ =2;

Choose a reason for hiding this comment

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

Исправь форматирование, нажми ctrl+alt+l хотя бы.

dataSend[dataSend.length - SET_SEQ] = SEQ;
dataSend[dataSend.length - SET_ACK] = ACK;
DatagramPacket pktSend = new DatagramPacket(dataSend, dataSend.length, inetAddress, PORT);
client.send(pktSend);

Choose a reason for hiding this comment

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

Всё ещё client.send(pktSend) два раза в коде, а мог бы быть один. Внеси в цикл.


byte[] buf = new byte[SIZE_BUF];
DatagramPacket reply = new DatagramPacket(buf, buf.length);
for (int i = STARTER_COUNT; i <= REPEAT_SENDS; i++) {

Choose a reason for hiding this comment

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

Тут даже IDE подсвечивает, что условие цикла не имеет смысла. Твой цикл всегда завершается изнутри, либо по break, либо по System.exit().

Тогда это либо должен быть ещё один while(true) (не советую), либо условие должно работать. Для этого можно, скажем, вынести переменную i наружу и дать ей имя, затем уже после цикла проверять, не равна ли она максимуму и выбрасывать ошибку.


byte[] buf2;
buf2 = reply.getData();
String answer = new String(buf2);

Choose a reason for hiding this comment

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

Дублирование кода между Client и Server. Можно в какой-нибудь Util класс вынести.

private static final boolean FLAG_FALSE=false;
public static void main(String[] args) throws IOException {
try (DatagramSocket server = new DatagramSocket(PORT, InetAddress.getLocalHost())) {

Choose a reason for hiding this comment

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

Однозначно не хватает хотя бы базового рукопожатия. Я прощу, что seq и ack не имеют здесь смысла, кроме длины, но пусть будет хотя бы проверка того, что общение между клиентом и сервером вообще возможно до того, как мы начнем отправлять основные пакеты.

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