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

First impl #21

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

First impl #21

wants to merge 4 commits into from

Conversation

kaifeur
Copy link

@kaifeur kaifeur commented Mar 1, 2019

No description provided.

@otopba otopba self-requested a review March 3, 2019 20:11
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.

Задание выполнено очень хорошо. Буквально пара замечаний

currentStudent = new Student("", "", false, 0);
}

private void setupEdit(String name, String fname, Boolean isMale, int pic) {
Copy link

Choose a reason for hiding this comment

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

fname это сокращение от чего?

Copy link
Author

Choose a reason for hiding this comment

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

family name


private void onDeleteClick() {
if (currentStudent == null) {
Toast.makeText(MainActivity.this, R.string.empty_student, Toast.LENGTH_SHORT).show();
Copy link

Choose a reason for hiding this comment

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

Можно просто this

Toast.makeText(MainActivity.this, R.string.wrong_enter, Toast.LENGTH_SHORT).show();
} else {
if (currentStudent == null || currentStudent.getFirstName().equals("")) {
students.add(currentStudent = new Student(name.getText().toString(), fname.getText().toString(), male.isChecked(), picId = getRandomPick(male.isChecked())));
Copy link

Choose a reason for hiding this comment

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

name.getText() иногда может вернуть null


static final class StudentViewHolder extends RecyclerView.ViewHolder {

private final CircleImageView pImageCircleImageView;
Copy link

Choose a reason for hiding this comment

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

pImageCircleImageView что означет буква p вначале?

Copy link
Author

Choose a reason for hiding this comment

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

profileImage, исправлю

@@ -3,7 +3,198 @@
xmlns:tools="http://schemas.android.com/tools"
Copy link

Choose a reason for hiding this comment

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

Очень большая вложенность. Не стоит вкладывать констрейнт в констрейнт

…xt.getText() usage (null case), some code improvements
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. Нет выделение активного элемента в RecyclerView
  2. Пол студента не меняется

@otopba
Copy link

otopba commented Mar 9, 2019

Работа принята.
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