implementation #11

kilinochi · 2019-02-28T06:56:19Z

Спустя три дня работы и изучения паттерна "Репозиторий"

otopba

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

Код стаил в java выглядит по-другому. Сейчас очень напоминает C
Если не заполнить какие-то поля и нажать “save” все сбрасывается. Это не удобно
Если менять уже созданного студента, то можно поставить ему пустые поля
Пожалуйста, удали из репозитория папку .idea. Это моя изначальная ошибка. Можно забрать изменения из моего репозитория, там исправлена эта проблема

otopba · 2019-03-03T20:54:01Z

app/src/main/java/ru/ok/technopolis/students/MainActivity.java

+
+    private void onStudentClick(Student student)
+    {
+        startActivityForResult(new Intent(this, StudentActivity.class).putExtra("Student", student), 2);


волшебные слова и числа лучше убирать в константы

otopba · 2019-03-03T20:54:39Z

app/src/main/java/ru/ok/technopolis/students/Repository/FemalePhotoRepository.java

+    private List <Integer> femalePhotos;
+
+    private FemalePhotoRepository() {
+        femalePhotos = new ArrayList(){{


Не хватает даймондов для описания типа дженерика

otopba · 2019-03-03T20:55:25Z

app/src/main/java/ru/ok/technopolis/students/Repository/MalePhotoRepository.java

+
+import ru.ok.technopolis.students.R;
+
+public class MalePhotoRepository implements PhotoRepository


Можно было не разделять фото на 2 репозитория, а сделать один с параметром. Или хотя бы унифицировать методы. Различие только в списке фоток, остальные методы одинаковы

otopba · 2019-03-03T20:56:34Z

app/src/main/java/ru/ok/technopolis/students/Repository/MalePhotoRepository.java

+
+public class MalePhotoRepository implements PhotoRepository
+{
+    private static final MalePhotoRepository ourInstance = new MalePhotoRepository();


можно просто instance или repository

otopba · 2019-03-03T20:57:08Z

app/src/main/java/ru/ok/technopolis/students/Repository/PhotoRepository.java

+public interface PhotoRepository
+{
+    Integer getPhotoInRepository ();
+    void putPhotoInRepository (Integer photo);


Почему Integer и не int?

otopba · 2019-03-03T20:57:42Z

app/src/main/java/ru/ok/technopolis/students/Repository/StudentDataRepository.java

+
+
+    @Override
+    public void delete(Student student)


Если идти по списку и удалять что-то из него можно упасть. В таких случаях следует использовать итератор

otopba · 2019-03-03T20:59:07Z

app/src/main/java/ru/ok/technopolis/students/Repository/StudentDataRepository.java

+    }
+
+    @Override
+    public List studentsOnRepository()


Лучше использовать дженерики и указывать тип списка

kilinochi · 2019-03-05T11:39:23Z

@otopba, здравствуйте, что сумел профиксил, написал в личку что осталось

kilinochi · 2019-03-05T12:14:37Z

fixed

otopba · 2019-03-05T12:24:47Z

Все еще можно сохранить студента без второго поля
Хотелось бы увидеть подсказки рядом с полями при создании студента, не понятно где имя, а где фамилия

otopba · 2019-03-05T12:25:32Z

app/src/main/java/ru/ok/technopolis/students/MainActivity.java

        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);
        setupRecyclerView();
        setupAddButton();
    }

-    private void setupAddButton()
-    {
+    private void setupAddButton(){


Форматирование стало лучше, но после закрывающейся скобки лучше оставлять пробел setupAddButton(){ -> setupAddButton() {

otopba · 2019-03-05T12:25:54Z

app/src/main/java/ru/ok/technopolis/students/MainActivity.java

-    {
-        startActivityForResult(new Intent(this, StudentActivity.class),1);
+    public void onClick(View v){
+        requestCode = 1;


Магические цифры и строки должны быть вынесены в константы

otopba · 2019-03-05T12:26:43Z

app/src/main/java/ru/ok/technopolis/students/MainActivity.java

    }

    @Override
-    protected void onActivityResult(int requestCode, int resultCode, @Nullable Intent data)
-    {
+    protected void onActivityResult(int requestCode, int resultCode, @Nullable Intent data){


Intent data помечен как Nullable, значит нельзя напрямую обращаться к его методам data.getSerializableExtra() . Нужно добавить проверку на нул

otopba · 2019-03-05T12:27:31Z

app/src/main/java/ru/ok/technopolis/students/Repository/PhotoRepository.java

-{
-    Integer getPhotoInRepository ();
+public interface PhotoRepository {
+    int getPhotoInRepository ();
    void putPhotoInRepository (Integer photo);


Здесь должен быть приметив, а не объект

otopba · 2019-03-05T12:28:57Z

app/src/main/java/ru/ok/technopolis/students/Repository/StudentDataRepository.java

-{
-    private static final StudentDataRepository ourInstance = new StudentDataRepository();
+public class StudentDataRepository implements StudentRepository {
+    private static final StudentDataRepository instance = new StudentDataRepository();


Если поле static final и есть необходимость использовать его где-то во вне, то правильнее седлать public static final и убрать геттер.

otopba · 2019-03-05T12:29:17Z

app/src/main/java/ru/ok/technopolis/students/Repository/StudentDataRepository.java

        return students;
    }

    @Override
-    public void add(Student student)
-    {
+    public void add(Student student){
        students.add(student);
    }



Следи за пустыми строками

otopba · 2019-03-05T12:29:58Z

app/src/main/java/ru/ok/technopolis/students/Repository/StudentDataRepository.java

-            if (students.get(i).getId().equals(student.getId())){
-                students.remove(i);
+    public void delete(Student student) {
+        Iterator <Student> iterator = students.iterator();


Проверяй как расставляются пробелы
Iterator iterator = students.iterator();
->
Iterator iterator = students.iterator();

otopba · 2019-03-05T21:31:53Z

Едет верстка
Нельзя поменять пол студента

otopba · 2019-03-05T21:32:51Z

app/src/main/java/ru/ok/technopolis/students/MainActivity.java

+        Student student;
+        String dataRequest;
+        switch (resultCode) {
+            case 1:


Константы создал, а чего не используешь? ACTION_CREATE_STUDENT_CLICK?

"NewStudent" это тоже константа

otopba · 2019-03-05T21:33:05Z

app/src/main/java/ru/ok/technopolis/students/Repository/FemalePhotoRepository.java

+            this.add(R.drawable.female_3);
+        }};
+    }
+


Лишняя строка

otopba · 2019-03-05T21:34:10Z

app/src/main/java/ru/ok/technopolis/students/StudentActivity.java

@@ -0,0 +1,130 @@
+package ru.ok.technopolis.students;


Пустые строки, константы

otopba · 2019-03-06T21:40:41Z

app/src/main/java/ru/ok/technopolis/students/MainActivity.java

@@ -21,6 +21,9 @@
    private final String dataResponse = "Student";
    private final int ACTION_ON_STUDENT_CLICK = 2;
    private final int ACTION_CREATE_STUDENT_CLICK = 1;
+    private final String dataRequestNewStudent = "NewStudent";


Константы лучше делать private static final

Имя константы пишется капсом через подчеркивание, как у ACTION_CREATE_STUDENT_CLICK

otopba · 2019-03-06T21:42:26Z

app/src/main/java/ru/ok/technopolis/students/StudentActivity.java

-                dataResponse = "StudentForDelete";
-                setResult(resultCode, new Intent().putExtra(dataResponse, currentStudent));
-                photoRepository.putPhotoInRepository(currentStudent.getPhoto());
+                setResult(DELETE_STUDENT_RESULT_CODE, new Intent().putExtra(RESPONSE_DELETE_STUDENT, student));


Смотри то такое result code в описании метода setResult. Это должны быть коды из набора:
* @see #RESULT_CANCELED
* @see #RESULT_OK
* @see #RESULT_FIRST_USER

otopba · 2019-03-06T21:42:58Z

app/src/main/java/ru/ok/technopolis/students/StudentActivity.java

+                    setResult(CREATE_STUDENT_RESULT_CODE, new Intent().putExtra(RESPONSE_CREATE_STUDENT, student));
+                    studentImageViewPhoto.setImageResource(student.getPhoto());
+                } else {
+                    Toast.makeText(StudentActivity.this, "No Photo in DataBase", Toast.LENGTH_LONG).show();


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

otopba · 2019-03-06T21:43:09Z

app/src/main/java/ru/ok/technopolis/students/controller/StudentActivityController.java

+    private StudentActivityController() {
+
+    }
+


Следи за лишними строками

otopba · 2019-03-06T21:44:00Z

app/build.gradle

@@ -28,6 +28,10 @@ dependencies {
    implementation 'com.android.support:recyclerview-v7:28.0.0'
    implementation 'com.android.support.constraint:constraint-layout:1.1.3'
    implementation  'com.getbase:floatingactionbutton:1.9.1'
+    implementation 'com.google.dagger:dagger-android:2.20'


Зачем добавил эти зависимости?

otopba · 2019-03-06T21:45:47Z

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

Молодец что быстро реагируешь на замечания
Не молодец что не внимательный

kilinochi added 2 commits February 28, 2019 09:54

implementation

3076a83

for pull req

023cabd

otopba self-requested a review March 3, 2019 20:49

otopba suggested changes Mar 3, 2019

View reviewed changes

otopba added the Needs work label Mar 3, 2019

kilinochi added 3 commits March 5, 2019 13:21

some fix

02f56f1

some fix

23ff535

some fix

830b128

kilinochi added 2 commits March 5, 2019 15:12

Merge branch 'master' of https://github.com/otopba/students_homework

2fd6761

some fix

189ada3

otopba suggested changes Mar 5, 2019

View reviewed changes

kilinochi added 4 commits March 5, 2019 17:21

some fix

1777846

some fix

36d1acb

some fix

0911b27

some fix

80dac44

otopba suggested changes Mar 5, 2019

View reviewed changes

kilinochi added 5 commits March 6, 2019 11:56

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

b4e133b

create Controller

3a368ba

fix controller

5cb93d9

set checkboxListener

d3da531

fix updatePhoto

9d03dc0

otopba reviewed Mar 6, 2019

View reviewed changes

otopba added done 9 points and removed Needs work labels Mar 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implementation #11

implementation #11

kilinochi commented Feb 28, 2019 •

edited

Loading

otopba left a comment

otopba Mar 3, 2019

otopba Mar 3, 2019

otopba Mar 3, 2019

otopba Mar 3, 2019

otopba Mar 3, 2019

otopba Mar 3, 2019

otopba Mar 3, 2019

kilinochi commented Mar 5, 2019 •

edited

Loading

kilinochi commented Mar 5, 2019

otopba commented Mar 5, 2019

otopba Mar 5, 2019

otopba Mar 5, 2019

otopba Mar 5, 2019

otopba Mar 5, 2019

otopba Mar 5, 2019

otopba Mar 5, 2019

otopba Mar 5, 2019

otopba commented Mar 5, 2019

otopba Mar 5, 2019

otopba Mar 5, 2019

otopba Mar 5, 2019

otopba Mar 6, 2019

otopba Mar 6, 2019

otopba Mar 6, 2019

otopba Mar 6, 2019

otopba Mar 6, 2019

otopba commented Mar 6, 2019


		import ru.ok.technopolis.students.R;

		public class MalePhotoRepository implements PhotoRepository

implementation #11

Are you sure you want to change the base?

implementation #11

Conversation

kilinochi commented Feb 28, 2019 • edited Loading

otopba left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kilinochi commented Mar 5, 2019 • edited Loading

kilinochi commented Mar 5, 2019

otopba commented Mar 5, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

otopba commented Mar 5, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

otopba commented Mar 6, 2019

kilinochi commented Feb 28, 2019 •

edited

Loading

kilinochi commented Mar 5, 2019 •

edited

Loading