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
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
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
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
and all the others that are used there are not.
We go to MyScheduleActivity.java and see how the fragment is used.
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.
in all adapters just pull findViewById.
although we all know how to ViewHolder
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
very often guys close the cursor in the loader, which is contraindicated - Android Devs is
one example
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.
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.
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
- 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. - 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();
- they walked the cursor completely in the UI
- closed the cursor
- 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.