I / O Schedule 2014: A Bad Example for Learning

    We are already used to the fact that the application for Google I / O is de facto the standard of application architecture, code writing and design.

    So this time, I decided to see what new things appeared in the application. With design, everything is clear, or rather, it is clear that people need to learn to make it “right” again. But I was more interested in the code - what is new there?

    But I did not see anything new, but realized that the application is absolutely not suitable as a visual aid for training novice developers.

    After a quick inspection of the code, this is a list of comments.

    Disclaimer : I do not pretend to be the ultimate truth; most comments around myschedule

    1. Clearly lack of structure in the UI package


    Why not separate each part into separate packages?
    You can consider this remark "far-fetched", but it seems to me that extra packages would not hurt here

    2. Loading data into MyScheduleActivity.java


    We find the updateData method that rakes in the data for all days at once, through AsyncTask

    why is this strange and wrong? there are several factors
    1. judging by the fact that they reload the data on onume, a Loader that would do it automatically would suit them . CursorLoader does
      not fit , you can use AsyncTaskLoader . Moreover, he would kensel loading if the user left the screen. AsyncTask cannot do this himself.
    2. in fact, they are storing data in the adapter. in this situation, it’s more logical to erase the data with the model in the activity (although I don’t like it either), create adapters as needed. and only then feed them a model.

    3. Awful MyScheduleAdapter.java


    In fact, this is a regular ArrayAdapter , but no, you have to write everything yourself and inherit from the ListAdapter . 5 screen getView method ! Really, google? How to read this canvas?

    It was clearly written by several people in the best traditions - “I do not want to refactor”, “I do not want to understand”. otherwise, it’s a mystery to me why it’s worth putting these 3 colors in the members used in 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);
    

    and all the others that are used there are not.

    4. Strange fragment usage - MyScheduleFragment


    We go to MyScheduleActivity.java and see how the fragment is used.

    @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]);
        }
    


    It turns out the guys needed a ViewPager with a ListView . They did not want to play with the PagerAdapter and ListView since FragmentPagerAdapter already exists and manages fragments perfectly. and the getListView and setListAdapter methods of the fragment are public, why not use it. (the trainee came to me with exactly such arguments when he saw the item “use fragments” in the task).

    What is the mistake? part of the fragment is directly managed from activity. although the idea is that the fragment should be autonomous.

    Solution : if the fragment can’t receive the data on its own (they are formed by one in one activity), then you need to define the interfaceProvider in the fragment, this activity implements the interface and provides data through it to the fragment. The fragment will receive its data. and put them in the adapter.
    But this is almost the same. what they do, the reader will object.
    Nearly! it's like eating cutlets with borsch, and not after. as a result, they will enter the stomach, but here is the process.

    What is the difference. The fragment is an autonomous unit with a list and adapter inside. he knows how to display data. the only problem is the data itself - there is none. it’s through this interface that we will receive them, and not they will stick them to us.

    here is another solution that is better than the first - the fragment can load data for itself! those. loader in the fragment itself. and loads as much data as needed.

    5. Adapters do not use ViewHolder


    in all adapters just pull findViewById.
    although we all know how to ViewHolder

    6. A bunch of unnecessary observers


    each class considers it their duty to hang an observator,

    comes to an obsurd - restart the loader from the observatory. although the loader itself must learn everything about data changes.
    Itself, if pronotifit necessary url. again, if. SessionsFragment.java

            mSessionsObserver = new ThrottledContentObserver(new ThrottledContentObserver.Callbacks() {
                @Override
                public void onThrottledContentObserverFired() {
                    onSessionsContentChanged();
                }
            });
            activity.getContentResolver().registerContentObserver(
                    ScheduleContract.Sessions.CONTENT_URI, true, mSessionsObserver);
    


    7. Misuse of loaders


    very often guys close the cursor in the loader, which is contraindicated - Android Devs is

    one example
    mTagMetadata = new TagMetadata(cursor);
    cursor.close();
    

    1. they walked the cursor completely in the UI
    2. closed the cursor
    3. then they will render tags (they will inflate vyyushki and cram into LinearLayout)

    To free the cursor, you can kill the loader. but if they need it to overload the data on time - then you need to make a custom AsyncTaskLoader + observer (here it will be appropriate).
    The loader will load the cruiser, go around it, close it and issue the model.

    8. Using inline string instead of constants


    you can search by code "_uri"

    Perhaps I find fault, perhaps the code survived several generations of developers. but you can still look at the services - there, too, 10-20 actions through a switch are understood.

    IMHO, a novice developer is learning from this application is not worth it.

    Also popular now: