Сам в андройде не профессионал, скорее любитель, но свои 5 копеек вставлю:
private static Context mContext;
@Override
public void onCreate() {
super.onCreate();
mContext = this;
}
Хз почему так, но это считается не камильфо. Сам так юзаю, и не одного краш-дампа не словил, но в android-сообществе это не приветствуется (надеюсь в комментах кто-то опишет почему). Принято передавать контекст.
В MainActivity методы для работ с меню можно было удалить, ибо, судя по всему они нигде не юзаются (и видимо остались от типового шаблона).
В RestClient идет присвоение в статическю переменную.
private static RestClient instance;
Я так понимаю ты делал синглтон. Хз приемлемо ли это в андройде, но я б сделал его как компонент приложения. В идеале вся эта фигня должна создаваться через DI (по карйней мере в бэкенд-разработке DI везде используется). Для андройда это библиотека Dagger 2 (сам еще не юзал, но выглядит многообещающе).
Плюс там же:
public static RestAPI get() {
if(instance == null) instance = new RestClient();
return instance.restAPI;
}
Этот метод по хорошему должен быть помечен как synchronized: если туда зайдут одновременно 2 потока, то может получиться что создадутся 2 RestClient'a. Тут конечно побоку - это вряд-ли что то сломает, но в крупных приложениях это ого-го какие проблемы создаст (я опять таки про бэкенд, ибо я больше по нему).
Хранить во так параметры аутентификации - тоже не есть хорошо, но это тестовое задание, так что побоку.
Теперь по фрагментам:
list = (RecyclerView) v.findViewById(R.id.list);
Ты присваиваешь свойствам фрагмента вьюхи, в onDestroyView не плохо бы их обнулить. А еще лучше, юзать butterknife.
Вызов запросов в фрагменте нынче не моден. По хорошему бы сделать какую-нить службу-обертку над retrofit (службу не в смысле андройд службу, а в смысле паттерн), которой ты говоришь загрузи-ка мне данные, и ловишь потом от нее сообщения (в этом тебе поможет otto от square). При показе фрагмента ты запрашиваешь данные и подписываешся на события от сервиса, при скрытии фрагмента - отписываешся.
В обработке ответов ты проверяешь
if (getActivity() == null) return;. Но этим ты проверяешь наличие активити, но не фрагмента, фрагмент может быть уничтожен, или его вьюха может быть уничтожена, или у активити может быть вообще другой фрагмент или еще что. Короче, это все приведет к крэшу.
onTaskClick: По хорошему, менять фрагменты должна активити, фрагмент списка просто должен сказать "выбран такая-то задача", и вызвать каллбэк активити. А уже активити принять решение что делать дальше. В официальной документации описано как это сделать (механизм каллбэков).
Реально смутил DetailsTaskFragment: для представления информации о одной задаче ты юзаешь RecyclerView. Имхо, это в корне не правильно. Я понимаю твою мотивацию, почему ты так сделал (экономия память на вьюхах и прочее). Но ты жестко задал структуру и тип отображения. Малейшее изменение требования отображения задачи, и весь этот код под удаление. Ну и инструмент неправильный: RecyclerView - это для списков, для больших списков, а задача - это не список, это сущьность. Если бы ты отказался от этой идеи, и заюзал обычный layout + DataBinding все было бы более удобно, изменяемо, и кода было бы раз в 10 меньше.
В TaskListAdapter у тебя примешана бизнес-логика, а именно сортировка. По хорошему ты должен делать сортировку в другом месте, например, в службе которая тебе отдает данные, или где то еще, но точно не в UI, которым является адаптер.
ItemViewHolder - там у тебя обработчик. Хз правильно это или нет, но в примерх гугла обработчик вешается в onBindViewHolder.
Так же не совсем понимаю (возможно просто задание не читал) метод addAll: почему бы просто не обновить список? Плюс опять это по большой части бизнес-логика. И что странно - не вижу notifyDataSetChanged().
В общем как-то так.
Средний
Простой
