ha5 #4
ha5 #4
Conversation
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
С функциональностью всё идеально, надо только код привести в порядок:
- Надо комментарии
- Надо юнит-тесты на всё, что в принципе можно оттестировать (вывод картинок и ввод с клавиатуры не надо, но там перемещение персонажей, генерацию карты и т.д., особенно всякие алгоритмически нетривиальные вещи типа поиска пути и областей видимости)
- Надо логирование основных действий
- Надо ещё поправить замечания по коду. Разделение на модель и представление можно не делать, это всё переписывать, но порефакторить иерархии наследования и в целом навести порядок всё-таки надо.
| import screens.Cell; | ||
| import start.MainFrame; | ||
|
|
||
| public class Amunation { |
There was a problem hiding this comment.
Надо ещё комментарии, как обычно
| @@ -0,0 +1,66 @@ | |||
| package characters; | |||
There was a problem hiding this comment.
Имя пакета неканоничноЪ. Должно быть доменное имя наоборот вначале
| public class Amunation { | ||
| private Cell[] cells = new Cell[3]; | ||
| private Item armore = null; | ||
| private Item weapon = null; |
| } | ||
|
|
||
| public int getSumDamage() { | ||
| int armoreDmg = armore == null ? 0 : armore.getDamage(); |
There was a problem hiding this comment.
Кстати, почему armore? Оно ж "armor"
| Item old = null; | ||
| switch (item.type()) { | ||
| case WEAPON: | ||
| cells[0].setItem(item); |
There was a problem hiding this comment.
Это тогда лучше делать не массивом, а отдельным классом, типа слотов снаряжения. Чтобы было сразу понятно в коде, какой шмот куда одевается.
task5/src/main/java/items/Item.java
Outdated
| } | ||
|
|
||
| public static Item generateWeapon(int level) { | ||
| Random rand = new Random(); |
There was a problem hiding this comment.
Random инициализируется системным временем (по крайней мере под виндой, под линуксом он может использовать /dev/random, но вроде как тоже не обязан), так что если насоздавать пачку рандомов в одну миллисекунду, они все будут возвращать одно и то же. Кроме того, чем чаще рандомом пользуются, тем более рандомными становятся значения, так что лучше вообще иметь один рандом на всё приложение и передавать его везде, где он нужен.
| private int itemCount = 0; | ||
| private final int size = 15; | ||
| private ImageIcon background = new ImageIcon("sprites/inventory.png"); | ||
| private Amunation amunation = new Amunation(); |
| return amunation; | ||
| } | ||
|
|
||
| public boolean isFool() { |
|
|
||
| public class StartScreen extends JPanel implements Screen { | ||
|
|
||
| JLabel jlabel = new JLabel(); |
| import screens.Screen; | ||
| import screens.StartScreen; | ||
| import sun.audio.AudioPlayer; | ||
| import sun.audio.AudioStream; |
There was a problem hiding this comment.
У меня нету такого. И вообще, мне кажется, он умер: https://stackoverflow.com/questions/22031701/sun-audio-player-and-sound-i-o
| import spbau2018.se.screens.AmunationCells; | ||
| import spbau2018.se.start.MainFrame; | ||
|
|
||
| public class Ammunation { |
There was a problem hiding this comment.
Комментарии-то так и не появились
| import spbau2018.se.start.MainFrame; | ||
|
|
||
| public class Ammunation { | ||
| AmunationCells cells = new AmunationCells(); |
No description provided.