Task5/hello there(евгений) #253
Conversation
| import java.util.Map; | ||
|
|
||
| public class Main { | ||
| private static List<Student> studends = new ArrayList<>(); |
There was a problem hiding this comment.
Константой может быть? Думаю, мы это поле никогда не меняем
Можно добавить final и поменять название на STUDENTS тогда
| main.studentsSearch("Java программист", 1); | ||
| } | ||
|
|
||
| public Map<Pair, List<Student>> getGroupsInfo(List<Student> studends) { |
There was a problem hiding this comment.
Я бы скорее назвал этот метод groupStudents, потому что он именно группирует, а не получает группы. Мелочь на самом деле, но чуть более выразительное название было бы, мне кажется
| studends.remove(new Student(name, faculty, year)); | ||
| } | ||
|
|
||
| public List<Student> getStudentsList(String faculty, int year) { |
There was a problem hiding this comment.
А этот метод я бы назвал findBy или findStudentsBy. Не стоит в названии писать слово List, потому что метод и так уже возвращает список - видно, что это список именно
Потому что иначе тоже кажется, будто мы их получаем откуда-то. А ведь на самом деле мы их просто фильтруем по переданным параметрам и достаем из общего списка. Находим, короче, подходящие. Поэтому find
| return list; | ||
| } | ||
|
|
||
| public void studentsSearch(String faculty, int year) { |
There was a problem hiding this comment.
Я не совсем понимаю, что этот метод делает, из названия
Во-первых, методы должны называться через глаголы. Потому что методы - это действия
Во-вторых, этот метод типа считает, сколько студентов есть в определенной группе. Именно так бы я его и назвал, потому что он на самом деле не ищет что-то, а печатает на экран, сколько студентов в данной группе
может быть, название printGroup подошло бы
| public void studentsSearch(String faculty, int year) { | ||
| int count = 0; | ||
| for (Student student : studends) { | ||
| if (student.getFaculty().equalsIgnoreCase(faculty) && student.getYear() == year) { |
There was a problem hiding this comment.
Мы тут проверяем факультет, игнорируя регистр символов, а в методе getStudentsList не игнорируем регистр. Логика должна быть консистентной, однородной. Либо то, либо другое правило должно быть применимо везде. Иначе у нас получается, что часть функционала похожего работает иначе в сравнении с другой частью, которая почти такая же
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { |
There was a problem hiding this comment.
А мы используем эти методы equals & hashCode у этого класса? Просто если нет, то их не нужно просто так переопределять, потому что они объем кода увеличивают)
2 participants
task_5