I/O Schedule 2014: плохой пример для обучения
Мы уже привыкли, что приложение для Google I/O дефакто стандарт архитектуры приложения, написания кода и дизайна.Вот и в этот раз, я решил посмотреть, что же нового появилось в приложении. С дизайном все понятно, точнее понятно, что людям нужно снова учится делать его «правильно». Но меня больше интересовал код — что же нового там есть?
Но ничего нового я не увидел, но осознал, что приложение абсолютно не годится как наглядное пособие для обучения начинающих разработчиков.
После быстрого осмотра кода получился вот такой список замечаний.Disclaimer: не претендую на истину в последней инстанции; большинство замечаний вокруг MySchedule
1. Явно не хватает структурированности в пакете UIПочему бы не выделить в отдельные пакеты каждую часть? Можно считать это замечание «притянутым за уши», но мне кажется, что лишние пакеты тут бы не помешали2. Загрузка данных в MyScheduleActivity.java Находим метод updateData который загребает данные для всех дней сразу, через AsyncTaskпочему это странно и неправильно? тут несколько факторов
судя по тому что они перезагружают данные на onresume им бы подошел Loader который делал бы это автоматически.Не подходит CursorLoader, можно юзать AsyncTaskLoader. Тем более он бы кенселил загрузку если юзер ушел с экрана. AsyncTask этого сам сделать не может. фактически они сторят данные в адаптере. в это ситуации, логичнее сторить данные моделькой в активити (хотя мне это не нравится тоже), создавать адаптеры по необходимости. и только тогда скармливать им модельку. 3. Ужасный MyScheduleAdapter.java По факту это обычный ArrayAdapter, но нет, надо все самим написать и наследоваться от ListAdapter. метод getView размеров в 5 экранов! Серьезно, google? как это полотно читать? Его явно писали несколько человек в лучших традициях — «не хочу рефакторить», «не хочу разбираться». иначе для меня загадка почему стоит класть в мемберы эти 3 цвета, которые используются в getView
mDefaultSessionColor = mContext.getResources ().getColor (R.color.default_session_color); mDefaultStartTimeColor = mContext.getResources ().getColor (R.color.body_text_2); mDefaultEndTimeColor = mContext.getResources ().getColor (R.color.body_text_3); , а все остальные, которые используются там же, нет.4. Странное использование фрагмента — MyScheduleFragment Идем в MyScheduleActivity.java и смотрим как юзается фрагмент. @Override public void onFragmentViewCreated (ListFragment fragment) { fragment.getListView ().addHeaderView ( getLayoutInflater ().inflate (R.layout.reserve_action_bar_space_header_view, null)); int dayIndex = fragment.getArguments ().getInt (ARG_CONFERENCE_DAY_INDEX, 0); fragment.setListAdapter (mScheduleAdapters[dayIndex]); fragment.getListView ().setRecyclerListener (mScheduleAdapters[dayIndex]); } Оказывается, ребятам нужен был ViewPager с ListView. Играться с PagerAdapter и ListView они не захотели т.к. FragmentPagerAdapter уже есть и менеджит фрагменты отлично. да еще и методы getListView и setListAdapter у фрагмента публичные, почему бы не юзать. (с точно такими аргументами ко мне приходил и практикант, когда увидел в задаче пункт «использовать фрагменты»).
Ошибка в чем? часть фрагмента напрямую менеджется из активити. хотя по задумке фрагмент должен быть автономным.
Решение: если фрагмнет не может самостоятельно получить данные (у них они формируются одним скопом в активити), то надо задефайнить интерфес Provider во фрагменте, этот активити реализует интерфейс и провайдит данные через него во фрагмент. фрагмнет получит свои данные. и покладет их в адаптер.Но это ведь почти тоже самое. что делают они, возразит читатель.Почти! это как котлеты кушать вместе с борщем, а не после. в результате они попадут в желудок, но вот процесс.
В чем разница. Фрагмет автономная едицница со списком и адаптерво внутри. он умеет отобразить данные. проблема только в самих данных — их нет. вот через этот интерфейс мы их и получим, а не нам их засунут.
вот еще решение, которое лучше первого — фрагмент умеет грузить данные для себя! т.е. лоадер в самом фрагменте. и грузит данных столько сколько надо.
5. В адаптерха не используются ViewHolder во всех адаптерах просто дергают findViewById.хотя все мы знаем как надо ViewHolder6. Куча не нужных обсерверов каждый класс считает своим долгом повесить обсервердоходит до обсурда — перезапускают лоадер из обсервера. хотя лоадер то и сам все должен узнавать об изменении данных.Сам-то сам, если пронотифаить нужные url. опять «если». SessionsFragment.java
mSessionsObserver = new ThrottledContentObserver (new ThrottledContentObserver.Callbacks () { @Override public void onThrottledContentObserverFired () { onSessionsContentChanged (); } }); activity.getContentResolver ().registerContentObserver ( ScheduleContract.Sessions.CONTENT_URI, true, mSessionsObserver); 7. Неправильное использование лоадеров очень часто ребята закрывают cursor в лоадере, что противопоказано — Android Devsодин из примеров
mTagMetadata = new TagMetadata (cursor); cursor.close (); они обошли курсор полностью в UI закрыли курсор потом будут рендерить теги (будут инфлейтить вьющки и напихать в LinearLayout) что бы освободить курсор можно задестроить лоайдер., но если он им нужен что бы перегружать данные вовремя — то нужно делать кастомный AsyncTaskLoader + обсервер (тут он будет уместен).Лоадер загрузит крусор, обойдет его, закроет и выдаст уже модельку.8. Использование inline string вместо констант можете поискать по коду »_uri«Возможно я и придираюсь, возможно код пережил несколько поколений разработчиков., но можно еще заглянуть в сервисы — там тоже по 10–20 акшенов через switch разбираются.
ИМХО, начинающему разработчику учится по этому приложению не стоит.
