Танчики в консоли, статья вторая: «Настало время всё переделать!»

[zCooler]

Репозитории то пустые :)

[koito_tyan]

Немного сглупила, поправила ссылку.

[koito_tyan]

Не уверенна, сейчас проверю)

[koito_tyan]

Убрала и подправила, спасибо за указание на ошибку)

[ser-mk]

Хех, успешного code review на Хабре

ну а так...

не торт

[koito_tyan]

Опишите пожалуйста что вам не понравилось (я хочу написать следующую статью, и по возможности исправить эту, в более лучшей версии)

[koito_tyan]

понятную и красочную* (заместо лучшей)
(истекло время редактирования)

[FadeToBlack]

> if (cooldown <= 0) { for_shot = true; cooldown = 10; }
habrahabr.ru/post/341198

[Vest]

Я тут решил прогнать ваш проект бесплатной PVS-Studio. Я взял ветку final-v1.3, до 4го коммита. И хотел бы обратить ваше внимание на предупреждения, что она мне показала:

V3010 — The return value of function 'CreateShot' is required to be utilized. Program.cs 241

PlayerState.CreateShot(Players[MY_ID], PlayerState.NewPosition_X(MyTank, '\0'), 3);

Ваш метод создаёт объект, возвращает его, но не использует.

И два предупреждения:
V3102 — Suspicious access to element of 'x' object by a constant index inside a loop. Program.cs 551, 559

for (int i = 0; i < x.Length - 1; i++) {
    Console.CursorLeft = x[0];
    Console.CursorTop = y[i];

В обоих случаях вы итерируете по «x», но индекс используете для «y».

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

p.s. ах да, строчки могут отличаться на 2, потому что для PVS-Studio пришлось добавить 2 строки комментария.

Andrey2008, надеюсь я вашу лицензию не нарушил. Я временно добавил шапку и убрал её.

[koito_tyan]

Всё хорошо, лицензию не завезли (не переживайте)

Спасибо за указание на ошибку, изменила строчки кода и сейчас обновлю код в рипозиторие )

[FadeToBlack]

Вот мне не ясно: зачем сразу статью писать? Можно же просто складывать их в папочку и никогда не публиковать, как это делал я, когда только научившись чему-нибудь брался писать статьи. Почему бы не дождаться, когда ваш код станет простым и ясным, чтобы быть хорошим примером, а не вот эти тысячи строк.

[koito_tyan]

Хорошо, но как я пойму простой код или нет, как можно оценить свой код не слушая критику?

Самооценивание и сравнение? Но эти методы почти не работают с новичками (и я тому, как видите пример (хотя все остальные проекты живут в ЯндексДиск (в папочке)).

[FadeToBlack]

1. Освоить создание своих типов с перегрузкой операторов, чтобы не писать по две строчки одинакового кода
2. Освоить функцию cos / sin (или написать функцию angle to dir), чтобы не писать switchcase для 4х случаев.

[koito_tyan]

Но смысл от косинуса или синуса, если мы используем всего 4 случая?

Это не WinForm версия…

[FadeToBlack]

угол определяет направление через cos/sin. лучше абстракции (и меньшее количество кода) сложно придумать.

[mayorovp]

Потому что вы сначала отказались от направления через перечисление в пользу угла под предлогом упрощения перехода к WinForm версии — а теперь этот переход усложняете.

[FadeToBlack]

3. Понять, что булевы выражения уже годятся, чтобы возвращать результат, а дополнительный код не требуется

if (startPosition.X > 0 && startPosition.Y > 0) return true;
    return false;

[koito_tyan]

Подумаю на этот счёт, спасибо )

[FadeToBlack]

4. Слишком много static. Для этого существуют синглтоны, в таком случае очень легко конвертить синглтон в обычный объект (когда потребуется сделать множество контекстов абстракции и так далее).

[koito_tyan]

Я не знаю синглтонов…

[FadeToBlack]

ну теперь то это не так

[FadeToBlack]

Жду еще пару статей на тему «настало время еще раз все переделать».

[koito_tyan]

Их будет много, пока не напишу сервер и не протестирую в полевых условиях )

[DarkerSigner]

Буду конечно занудой, но я думаю вам стоит все же разбить код на несколько файлов, а то получается солянка и банально не удобно читать.

[koito_tyan]

Да, вы правы, как только я допишу ещё два метода (на загрузку) я сделаю нормально распределение (смотрите в рипозитории изменения). Спасибо за ваше 'занудство', ведь оно помогает и мне, и остальным )

[ForNeVeR]

Это хорошая и правильная идея — предоставлять свой код для оценки сообществом (даже если вы новичок, в этом нет ничего зазорного). Другой разговор, что сообщество Хабра, возможно, для этого не лучшим образом подходит (на этом ресурсе люди всё-таки чаще ожидают каких-то законченных материалов для специалистов среднего и высокого уровня). Мне кажется, что куда больше фидбэка вы сможете получить, если обратитесь в места с более высокой концентрацией .NET-разработчиков.

Вот тут перечислено несколько .NET-сообществ в Telegram, приходите :)

[koito_tyan]

Обязательно загляну к вам как только будет свободное время (немного много учёбы) )

[mayorovp]

Зачем вы вообще проверяете направление выстрела в методе ForgetTheWay, если все прошлые позиции уже записаны в списках x_way и y_way?

[koito_tyan]

Я думала что так будет понятнее что удалять, протестирую другие варианты и изменю, спасибо за поправку )

[mayorovp]

Кстати, вы совершенно напрасно выбрали UDP-сокеты. Эти сокеты подходят только если вы делаете свой протокол с контролем потерь пакетов (совершенно не ваш случай) или же если несколько потерянных пакетов ничего не изменят.

У вас потеря пакетов с выстрелами порушит всю игру. Не надо так делать, используйте TCP.

[koito_tyan]

Я придумала идею получше и уже обсуждаю её, если хотите можете присоединится к нам, вот мой vk: vk.com/unicode_72

[mayorovp]

По поводу многопоточности: что-то я не вижу у вас в коде никаких блокировок. А они нужны раз уж у вас доступ к одним и тем же объектам идет сразу из кучи потоков!

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

А дальше надо думать как синхронизировать «клавиатурный поток» с сетевым. Или наставить локов на каждую операцию, или складывать все события в блокирующую очередь, которую будет разбирать третий поток.

[koito_tyan]

Спасибо за поправку, вы помогли сделать мой код лучше, допилю код и выложу в рипозиторий )

[azhira]

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

1. Среди большинства .NET разработчиков принят CamelCase стиль именования классов/методов. Если он Вам не мил, используйте другой, но придерживайтесь его везде в рамках одного проекта. Сейчас же можно встретить идущие подряд методы Event_work, MoveShot и from_to_key.
2. По возможности выбирайте названия переменных так, чтобы их назначение было понятным без комментариев. Вот так точно не надо (я сначала подумала, что tasc — это опечатка):
   

   Task tasc = new Task(() => { Event_listener(); });
   Task task = new Task(() => { To_key(); });

3. Если же комментируете код, то пишите, почему добавлена та или иная строка, а не что она делает. И не забывайте актуализировать:
   

   // Создаем экземпляр класса Position, с координатами 2, 2
   var startPosition = new Position(10, 5);

4. Про разбиение кода на несколько файлов уже говорили. Можно было хотя бы #region использовать, чтобы структурировать.

Я понимаю, сейчас это все выглядит несущественным, но нужно прививать себе хорошие привычки.
Извините, если высказалась резковато. Вы смелая и, надеюсь, упорная. Интересно посмотреть, что получится в итоге.

[koito_tyan]

Всё хорошо. Я люблю критику )

А по поводу кода… я изменю его как только будет готово взаимодействие с сервером перед началом игры (уже пишу третью функцию), все изменения есть в рипозитории )