ВСЕ ИССУЕСЫ

[martinkomitsky]

Привет!

Подробный отчет ниже. Считаю его непредвзятым, в случае несогласия рекомендую обратиться к любому опытному разработчику Android, и поэтому вступать в дискуссию и обсуждать, как можно исправить указанные ошибки, я не собираюсь. Если что-то не понятно, все ответы можно найти в Google, так как ошибки тривиальные.

Ссылка на проект

Критически важные ошибки:

1. Проект не собирается из консоли с помощью команды gradle build. Это очень важно, так как в реально мире большинство проектов собираются автоматически билд-сервером без использования Android Studio. Проблема сборки на самом деле состоит в том, что проект не проходит синтаксическую проверку Lint. Проблема в зависимых библиотеках retrofit и okio. Для них необходимо настроить правильное исключение. Проблема описана здесь. Мое мнение — если программист решается использовать сторонние библиотеки, то он должен четко понимать как ими пользоваться и как настраивать сборку с их использованием.
2. Вторая проблема сборки — использование API версии 21, в то время как в настройках проекта стоит минимальная версия API 15, а в задании указано, что приложение должно работать минимум с API 16

NewApi: Calling new methods on older versions
../../src/main/res/drawable/ripple.xml:2: <ripple> requires API level 21 (current min is 15)
1 <?xml version="1.0" encoding="utf-8"?>
2 <ripple xmlns:android="http:https://schemas.android.com/apk/res/android"
3 android:color="#ffa0a0a0">
4 <item android:id="@android:id/mask">
../../src/main/res/drawable/ripple.xml:4: @android:id/mask requires API level 21 (current min is 15)
1 <?xml version="1.0" encoding="utf-8"?>
2 <ripple xmlns:android="http:https://schemas.android.com/apk/res/android"
3 android:color="#ffa0a0a0">
4 <item android:id="@android:id/mask">
5 <shape android:shape="rectangle">
6 <solid android:color="#ffa0a0a0"/>

Это приводит к тому, что приложение на телефонах с версией Android меньше 5.0, падает при попытке отобразить список контактов:

E/AndroidRuntime: FATAL EXCEPTION: main
android.view.InflateException: Binary XML file line #2: Error inflating class android.widget.RelativeLayout
at android.view.LayoutInflater.createView(LayoutInflater.java:613)
at com.android.internal.policy.impl.PhoneLayoutInflater.onCreateView(PhoneLayoutInflater.java:56)
at android.view.LayoutInflater.onCreateView(LayoutInflater.java:660)
at android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:685)
at android.view.LayoutInflater.inflate(LayoutInflater.java:466)
at android.view.LayoutInflater.inflate(LayoutInflater.java:396)
at android.view.LayoutInflater.inflate(LayoutInflater.java:352)
at ru.mail.tp.callbackpal.contacts.ContactsAdapter.onCreateViewHolder(ContactsAdapter.java:30)
at ru.mail.tp.callbackpal.contacts.ContactsAdapter.onCreateViewHolder(ContactsAdapter.java:20)

3. Третья серьезная проблема — если же приложение запустить на Android 6.0 или 7.0 то оно все равно упадет, так как для получения списка контактов необходимо получить Runtime Permission на доступ к контактам. Код запроса Permission в проекте вроде как есть, но он не вызывается и не понятно, рабочий он или же нет. В итоге после проверки номера телефона и при следующем запуске приложение падает и исключением:

E/AndroidRuntime: FATAL EXCEPTION: main
Process: ru.mail.tp.callbackpal, PID: 2012
java.lang.RuntimeException: Unable to start activity ComponentInfo{ru.mail.tp.callbackpal/ru.mail.tp.callbackpal.ContactsListActivity}: java.lang.SecurityException: Permission Denial: opening provider com.android.providers.contacts.ContactsProvider2 from ProcessRecord{8e350ef 2012:ru.mail.tp.callbackpal/u0a70} (pid=2012, uid=10070) requires android.permission.READ_CONTACTS or android.permission.WRITE_CONTACTS
at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2646)
at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2707)
at android.app.ActivityThread.-wrap12(ActivityThread.java)
at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1460)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loop(Looper.java:154)
at android.app.ActivityThread.main(ActivityThread.java:6077)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:865)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:755)
Caused by: java.lang.SecurityException: Permission Denial: opening provider com.android.providers.contacts.ContactsProvider2 from ProcessRecord{8e350ef 2012:ru.mail.tp.callbackpal/u0a70} (pid=2012, uid=10070) requires android.permission.READ_CONTACTS or android.permission.WRITE_CONTACTS
at android.os.Parcel.readException(Parcel.java:1683)
at android.os.Parcel.readException(Parcel.java:1636)
at android.app.ActivityManagerProxy.getContentProvider(ActivityManagerNative.java:4169)
at android.app.ActivityThread.acquireProvider(ActivityThread.java:5434)
at android.app.ContextImpl$ApplicationContentResolver.acquireUnstableProvider(ContextImpl.java:2267)
at android.content.ContentResolver.acquireUnstableProvider(ContentResolver.java:1515)
at android.content.ContentResolver.query(ContentResolver.java:514)
at android.content.ContentResolver.query(ContentResolver.java:472)
at ru.mail.tp.callbackpal.ContactsListActivity.getAllContactsList(ContactsListActivity.java:39)
at ru.mail.tp.callbackpal.ContactsListActivity.onCreate(ContactsListActivity.java:27)
at android.app.Activity.performCreate(Activity.java:6664)
at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1118)
at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2599)
at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2707) 
at android.app.ActivityThread.-wrap12(ActivityThread.java) 
at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1460) 
at android.os.Handler.dispatchMessage(Handler.java:102) 
at android.os.Looper.loop(Looper.java:154) 
at android.app.ActivityThread.main(ActivityThread.java:6077) 

Ошибки с высокой степенью важности:

1. Приложение никак не отслеживает состояние сети. Если включить режим полета и запустить приложение, оно никак не проинформирует пользователя о том, что в данный момент оно работать не может. Можно ввести номер телефона, нажать кнопку Validate Phone, то через несколько секунд в поле ввода кода появится слово "Some" и информация о том, что код неверный. При этом попытка нажатия на ссылку Retry ни к чему не приводит и изменить номер введенного телефона в этот момент нельзя, даже нажав кнопку "Домой" и повторно запустив приложение.
2. Файл SplashScreenActivity.java. Вызов getSupportActionBar().hide(); даже AndroidStudio подсвечивает желтым и дает пояснение, что ф-ция getSupportActionBar() может вернуть null и это надо проверять, иначе это может привести к падению.
3. Файл SplashScreenActivity.java. Переменная backPressed используется в двух потоках (UI и ThreadSleep) однако доступ к ней не защищен, что может привести к проблеме, когда один поток не видит изменение закэшированных данных другого потока. Такие переменные стоит объявлять как минимум volatile, а лучше использовать Atomic-классы
4. Файл SplashScreenActivity.java. Внутри потока ThreadSleep хранится жестка ссылка на контекст (активити), что может привести утечке ресурсов. Причем, что странно, тут же ниже в классе ActivityStarter уже хранится ссылка через WeakReference. То ли разные люди код писали, то ли нет понимания для чего это надо.
5. Файл LoginActivity.java. Код TelephonyManager tManager = (TelephonyManager) getSystemService(Context.TELEPHONY_SERVICE); может вернуть null на некоторых сборках (например китайский планшет без радио-модуля), что приведет к падению приложения
6. Файл UserLoginTask.java. В методе doInBackground вызывается метод enqueue, который добавляет в очередь запрос. Причем этот метод асинхронный, это означает, что он вернет выполнение еще раньше, чем выполнится запрос. После чего метод doInBackground вернет true и в методе onPostExecute вызовется mActivityRef.get().showProgress(false); т.е. спрячется ProgressBar, в то время, как на очень медленном интернете запрос еще может выполнятся. Если уж программист использует сторонние библиотеки, то он должен четко понимать, как они работают.
7. Файл UserLoginTask.java. Переменная mActivityRef объявлена как WeakReference, однако везде вызов mActivityRef.get() не проверяется на возврат null. Такое использование неправильно и может привести к падению. Странно что в классе SplashScreenActivity.java WeakReference используется корректно. У меня все больше подозрений, что код писали разные люди. Или же код брался с разных сайтов. Даже Android Studio подсвечивает желтым цветом вызов этих методов и указывает, что они могут вернуть null.
8. Файл ContactsListActivity.java. В методе onCreate вызывается метод getAllContactsList который вычитывает все контакты в UI потоке. Если у пользователя большое число контактов (+10000), приложение будет на этом моменте тормозить, пользователь будет долгое время видеть белый экран и в лучшем случае в итоге увидит список, а в худшем приложение будет закрыто по ANR.
9. Файл ContactsListActivity.java. Вызов метода contentResolver.query может вернуть null, однако это не проверяется. Даже Android Studio подсвечивает желтым цветом вызов этих методов и указывает, что они могут вернуть null.
10. Файл ContatcsListActivity.java. Вызов метода contentResolver.query возвращает объект Cursor, который должен быть закрыт, после окончания работы с ним с помощью метода close().
11. Файл ContactsListActivity.java. Вызов Cursor phoneCursor = contentResolver.query так же может вернуть null.

Ошибки средней важности:

1. Файл LoginActivity.java. Переменные mAuthTask, validationPin, mPasswordView объявлены как паблик, что нарушает концепцию инкапсуляции.
2. Файл ContatcsListActivity.java. List<Contact> contactList = new ArrayList(); используется создание массива объектов класса Object. Правильная запись List<Contact> contactList = new ArrayList<>();

Незначительные проблемы:

1. SplashScreenActivity.java. Мне не понятно, для чего поля backPressed и isValidated объявлены как static. Так же не понятно, почему backPressed объявлен как простой тип boolean, а isValidated уже как класс Boolean.
2. LoginActivity.java. Вызов getApplicationContext().getSharedPreferences("ValidationData", MODE_PRIVATE);. Ключи, как и любые другие захардкоженные значения необходимо выносить в константы. Это касается и вызовов такого плана editor.putString("phone", "+7" + mPhoneView.getUnmaskedText());
3. Так же хочу заметить что код очень грязный — много закомментированных участков и много кода, который не используется. Из-за этого не очень понятно, что же хотел сделать программист.

Резюме:

Выполненное задание представляет из себя очень простое приложение, которое состоит буквально из трех Activity. Однако его невозможно собрать. Все сложные участки (сетевые запросы, кастомные view-элементы) реализованы с помощью сторонних библиотек. При этом основной функционал приложения — отображение списка контактов, которые можно провалидировать, не работает из-за падения приложения практически на любой версии Android. Так же несмотря на указания в задании, в приложение не использует Fragments API. В задании на сайте было указано, что приложение не должно быть стыдно выложить в Google Play. Могу сказать, что если выложить такое приложение, оно не наберет оценку выше 2 баллов.

Не реализованы следующие пункты задания 2, 4, 6, 7, 8, 9, 10, 11. Так же не реализован ни один из дополнительных пунктов задания.