Skip to content
This repository has been archived by the owner on Mar 1, 2021. It is now read-only.

Version 1.0 Loginov #1

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Version 1.0 Loginov #1

wants to merge 9 commits into from

Conversation

l0gark
Copy link

@l0gark l0gark commented Feb 22, 2019

No description provided.

Добавление студента, RecyclerView студентов, в рзметке использовал ConstraintLayout. Общение с пользователем по средством диалогов, можно для студента выбрать фото из галереи(можно не выбирать)
убрал из Student Serializable
@otopba otopba self-requested a review March 3, 2019 18:22
Copy link

@otopba otopba left a comment

Choose a reason for hiding this comment

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

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

Общие замечания:

  1. Если вводить очень длинные имя/фамилию, едет верстка
  2. Можно сохранить человека без пола
  3. Если введены поля лучше сразу показать ошибку, чем закрывать окно а потом уже информировать

app/src/main/res/layout/activity_main.xml Outdated Show resolved Hide resolved
@@ -0,0 +1,104 @@
<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
Copy link

Choose a reason for hiding this comment

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

Можно сократить вложенность, если сделать родителем констрейнт

app/src/main/res/layout/item_student.xml Outdated Show resolved Hide resolved
@l0gark
Copy link
Author

l0gark commented Mar 3, 2019

Везде поставил видимость.
Удалил лишние пустые строки.
Coordinator заменил на Frame.
Создал View диалога в коде.
Теперь длинные слова не ломают верстку.
При некорректных данных , пользователь остаётся в том же окне.
Нельзя создать студента без пола.

Copy link

@otopba otopba left a comment

Choose a reason for hiding this comment

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

Нужно еще доработать код

listener.add(name, lastName, maleGender, bitmap);
}
} else {
OnUpdateStudentListener listener = (OnUpdateStudentListener) context;
Copy link

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

Не понял, что Вы имеете ввиду

Copy link

Choose a reason for hiding this comment

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

Лисенер это отдельный класс. Не стоит расчитывать что context всегда будет наследован от него. Диалог должен передавать свои результаты лисенеру, а не контексту

@l0gark
Copy link
Author

l0gark commented Mar 5, 2019

Всё исправил кроме передачи листенера


public class MainActivity extends AppCompatActivity implements StudentAdderDialog.OnAdderStudentListener, StudentAdderDialog.OnUpdateStudentListener {

private String[] mansFirstNames;
Copy link

Choose a reason for hiding this comment

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

Это решил не исправлять?

@otopba
Copy link

otopba commented Mar 5, 2019

При изменении пола аватарка не меняется

@otopba
Copy link

otopba commented Mar 5, 2019

@LoginovArkadiy

Сделай метод setListener, который будет устанавливать лисенер у диалога

Copy link

@otopba otopba left a comment

Choose a reason for hiding this comment

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

Работа принята.
9 балов

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants