-
Notifications
You must be signed in to change notification settings - Fork 17
Homework #6
base: master
Are you sure you want to change the base?
Homework #6
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Работа выполнена хорошо.
Есть небольшие замечания
ball = findViewById(R.id.activity_main_ball); | ||
hoop = findViewById(R.id.activity_main_hoop); | ||
|
||
final ConstraintLayout activityMain = findViewById(R.id.activity_main); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это не активити, название не соответствует
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Исправила
} | ||
|
||
public boolean AnimatorTrack(final float X, final float Y, final float xHoop, final float yHoop, final float targetSize) { | ||
ValueAnimator aniView = ValueAnimator.ofFloat(0, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aniView?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Опечаталась, поправила
ball.setTranslationX((float) ((1 - Math.cos(angle)) * X)); | ||
ball.setTranslationY((float) (-Math.sin(angle) * Math.abs(Y))); | ||
|
||
if ((Math.abs(xHoop - x) <= targetSize) && (Math.abs(yHoop - y) <= targetSize)) hit = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Лучше всегда ставить фигурные скобки
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Поставила
float y = ball.getY(); | ||
int rotation = 50 + (int) (Math.random() * 101); | ||
|
||
float value = (Float) valueAnimator.getAnimatedValue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Зачем тут каст к объекту Float
? можно просто float
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Исправила
switch (hits) { | ||
case 1: | ||
star = findViewById(R.id.imageStar1); | ||
star.setImageDrawable(getResources().getDrawable(R.drawable.ic_grade_red_24dp)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Можно один раз считать drawable и потом использовать. А не считывать каждый раз
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Исправила
ImageView star; | ||
switch (hits) { | ||
case 1: | ||
star = findViewById(R.id.imageStar1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ну нужно каждый раз делать findViewById. Можно один раз найти вью и сохранить ссылку
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Исправила
android:id="@+id/activity_main_player" | ||
android:layout_width="76dp" | ||
android:layout_height="131dp" | ||
android:layout_marginLeft="50dp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
стоит добавить layout_marginStart
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Добавила
android:layout_marginTop="8dp" | ||
app:layout_constraintStart_toStartOf="parent" | ||
app:layout_constraintTop_toTopOf="parent" | ||
app:srcCompat="@drawable/ic_grade_black_24dp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Посмотри ворнинг от андроид студии:
To use VectorDrawableCompat, you need to make two modifications to your project. First, set android.defaultConfig.vectorDrawables.useSupportLibrary = true in your build.gradle file, and second, use app:srcCompat instead of android:src to refer to vector drawables.
app:layout_constraintStart_toStartOf="parent" /> | ||
|
||
<ImageView | ||
android:id="@+id/imageStar1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Названия должны быть однотипные. В нашем случае, через подчеркивание
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Исправила
@@ -2,12 +2,152 @@ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Стоит сохранять аниматоры и останавливать их при уничтожении активити
Нет кастомного вью |
app/src/main/java/ru/ok/technopolis/basketball/MainActivity.java
Outdated
Show resolved
Hide resolved
@@ -59,27 +60,27 @@ public boolean onTouch(View v, MotionEvent event) { | |||
} | |||
|
|||
public boolean AnimatorTrack(final float X, final float Y, final float xHoop, final float yHoop, final float targetSize) { | |||
ValueAnimator aniView = ValueAnimator.ofFloat(0, 1); | |||
ValueAnimator anyView = ValueAnimator.ofFloat(0, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тоже странно название. Почему стало any?
float angle = value * 3.14f; | ||
|
||
ball.setRotation(rotation * value); | ||
ball.setTranslationX((float) ((1 - Math.cos(angle)) * X)); | ||
ball.setTranslationY((float) (-Math.sin(angle) * Math.abs(Y))); | ||
|
||
if ((Math.abs(xHoop - x) <= targetSize) && (Math.abs(yHoop - y) <= targetSize)) hit = true; | ||
if ((Math.abs(xHoop - x) <= targetSize) && (Math.abs(yHoop - y) <= targetSize)) { hit = true;} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Лучше писать на следующей строчке то, что находится внутри фигурных скобок. Можно использовать автоформатирование
ImageView star; | ||
Drawable redStar = getResources().getDrawable(R.drawable.ic_grade_red_24dp); | ||
Drawable blackStar = getResources().getDrawable(R.drawable.ic_grade_black_24dp); | ||
ImageView star1=findViewById(R.id.image_Star1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Методы должны называться с маленьких букв
- Не нужно каждый раз считывать ресурсы, можно счиать родин раз
- Нейминг ресурсов должен включать только маленькие буквы (image_Star2 и тп)
break; | ||
case 5: | ||
star = findViewById(R.id.imageStar5); | ||
star.setImageDrawable(getResources().getDrawable(R.drawable.ic_grade_red_24dp)); | ||
star5.setImageDrawable(redStar); | ||
hits = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hits не используется
Изначально ani было от animator. Затем я забыла что это было так и подумала, что опечаталась в слове any, не вникая в код. И вот вспомнила и назвала полностью AnimatorValue. Всё остальное исправила |
До сих пор нет кастомного вью |
Работа принята |
No description provided.