Igor Lopatinskiy #23
Igor Lopatinskiy #23
Conversation
| @Override | ||
| public void onClick(View v) { | ||
| clearFields(); | ||
| adapter.setActiveStudent(adapter.NO_STUDENT); |
There was a problem hiding this comment.
NO_STUDENT статичное поле, можно обращаться прямо через класс
| } | ||
|
|
||
| private void saveStudent() { | ||
|
|
There was a problem hiding this comment.
Пофиксил
Казалось, что так более читабельно)
|
|
||
| private void saveStudent() { | ||
|
|
||
| String firstNameStr = firstName.getText().toString(); |
|
|
||
| private void saveStudent() { | ||
|
|
||
| String firstNameStr = firstName.getText().toString(); |
There was a problem hiding this comment.
Нужно ли в названии переменно пояснять что это стринг?
There was a problem hiding this comment.
У меня уже есть переменная firstName, в которой хранится само поле ввода
Если не пояснять, что это стринг, то придётся назвать ту переменную firstNameField
В итоге так или иначе нужно пояснять
Не знаю, как лучше сделать)
| @@ -4,13 +4,13 @@ | |||
|
|
|||
| private String firstName; | |||
| private String secondName; | |||
| private boolean maleGender; | |||
| private boolean isMale; | |||
There was a problem hiding this comment.
обычно называют переменную male, что бы геттер можно было назвать isMale
С твоим названием, сеттер должен иметь название не setMale а setIsMale
| @@ -45,4 +45,11 @@ public int getPhoto() { | |||
| public void setPhoto(int photo) { | |||
| this.photo = photo; | |||
| } | |||
|
|
|||
| public void copy(Student student) { | |||
| this.firstName = student.firstName; | |||
There was a problem hiding this comment.
Не забывай что присваивая значение объекта ты не копируешь его, а лишь устанавливаешь ссылку
There was a problem hiding this comment.
Пофиксил, но не уверен что правильно
|
|
||
| public static final int NO_STUDENT = -1; | ||
| private MainActivity context; | ||
| private List<Student> students = new LinkedList<>(); |
There was a problem hiding this comment.
Делал LinkedList опираясь на то, что студентов будут часто удалять
Думаю, разумнее будет всё же ArrayList, так как чаще всего элементы достаются из списка, чем удаляются
Переделал
| public class StudentAdapter extends RecyclerView.Adapter<StudentAdapter.StudentViewHolder> { | ||
|
|
||
| public static final int NO_STUDENT = -1; | ||
| private MainActivity context; |
There was a problem hiding this comment.
Плохой паттерн передавать активити в адаптер. Для работы с активити у тебя есть лисенеры
| @@ -0,0 +1,13 @@ | |||
| package ru.ok.technopolis.students; | |||
| @@ -1,9 +1,134 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <android.support.constraint.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android" | |||
| <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android" | |||
There was a problem hiding this comment.
Большая вложенность. Чем меньше вложенность тем лучше. Пробуй использовать более сложные лейауты
| @@ -45,4 +54,12 @@ public int getPhoto() { | |||
| public void setPhoto(int photo) { | |||
| this.photo = photo; | |||
| } | |||
|
|
|||
| public void copy(Student student) { | |||
| this.firstName = new String(student.firstName); | |||
| } | ||
|
|
||
| private void bind(Student student) { | ||
| if (student.isSelected()) |
| @@ -1,9 +1,134 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <android.support.constraint.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android" | |||
| <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android" | |||
|
Фотография человека при редактировании не совпадает с фото из списка
|
|
Работа принята |
it works