-
Notifications
You must be signed in to change notification settings - Fork 1
Interview antonio improved #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,4 +18,6 @@ | |
| </activity> | ||
| </application> | ||
|
|
||
| <uses-permission android:name="android.permission.INTERNET" /> | ||
|
|
||
| </manifest> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| package android.coding.interview.makeitawesome.fragment; | ||
|
|
||
| import android.coding.interview.makeitawesome.R; | ||
| import android.coding.interview.makeitawesome.model.Album; | ||
| import android.graphics.Bitmap; | ||
| import android.os.Bundle; | ||
| import android.support.annotation.Nullable; | ||
| import android.support.v4.app.Fragment; | ||
| import android.view.LayoutInflater; | ||
| import android.view.View; | ||
| import android.view.ViewGroup; | ||
| import android.widget.ImageView; | ||
|
|
||
| import com.bumptech.glide.Glide; | ||
| import com.bumptech.glide.request.animation.GlideAnimation; | ||
| import com.bumptech.glide.request.target.SimpleTarget; | ||
|
|
||
|
|
||
| public class DetailFragment extends Fragment { | ||
|
|
||
| public static Fragment newInstance(Album album) { | ||
| DetailFragment detail = new DetailFragment(); | ||
| detail.setAlbum(album); | ||
| detail.setRetainInstance(true); | ||
| return detail; | ||
| } | ||
|
|
||
| public Album getAlbum() { | ||
| return album; | ||
| } | ||
|
|
||
| public void setAlbum(Album album) { | ||
| this.album = album; | ||
| } | ||
|
|
||
| Album album; | ||
| ImageView mImage; | ||
|
|
||
| @Nullable | ||
| @Override | ||
| public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) { | ||
| View v = inflater.inflate(R.layout.picture_detail, container, false); | ||
| mImage = (ImageView) v.findViewById(R.id.item_icon); | ||
| showImage(); | ||
| return v; | ||
| } | ||
|
|
||
|
|
||
| public void showImage() { | ||
|
|
||
| Glide.with(getActivity()).load(album.getUrl()).asBitmap().into(new SimpleTarget<Bitmap>(350, 350) { | ||
| @Override | ||
| public void onResourceReady(Bitmap resource, GlideAnimation glideAnimation) { | ||
| mImage.setImageBitmap(resource); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,11 @@ | ||
| package android.coding.interview.makeitawesome.fragment; | ||
|
|
||
| import android.app.ProgressDialog; | ||
| import android.coding.interview.makeitawesome.R; | ||
| import android.coding.interview.makeitawesome.adapter.PhotosAdapter; | ||
| import android.coding.interview.makeitawesome.model.Album; | ||
| import android.coding.interview.makeitawesome.networking.AlbumEvent; | ||
| import android.coding.interview.makeitawesome.networking.RestClient; | ||
| import android.os.Bundle; | ||
| import android.support.annotation.Nullable; | ||
| import android.support.v4.app.Fragment; | ||
|
|
@@ -11,6 +15,13 @@ | |
| import android.view.View; | ||
| import android.view.ViewGroup; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import de.halfbit.tinybus.Subscribe; | ||
| import de.halfbit.tinybus.TinyBus; | ||
| import de.keyboardsurfer.android.widget.crouton.Crouton; | ||
| import de.keyboardsurfer.android.widget.crouton.Style; | ||
|
|
||
| /** | ||
| * This is Your TASK:<br> | ||
| * This is a fragment where you need to show list of pictures and details fetched from API<br> | ||
|
|
@@ -19,19 +30,78 @@ | |
| public class PicturesFragment extends Fragment { | ||
|
|
||
| public static Fragment newInstance() { | ||
| return new PicturesFragment(); | ||
| PicturesFragment pictures = new PicturesFragment(); | ||
| pictures.setRetainInstance(true); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not a good practice, but you can't completely blame someone for using it. At least, in this version he worries about the lifecycle. |
||
| return pictures; | ||
| } | ||
|
|
||
| RecyclerView rv; | ||
| ProgressDialog mProgressDialog; | ||
| List<Album> mData; | ||
| boolean loading = false; | ||
| private TinyBus mBus; | ||
|
|
||
| @Nullable | ||
| @Override | ||
| public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) { | ||
| RecyclerView rv = (RecyclerView) inflater.inflate(R.layout.pictures_list_fragment, container, false); | ||
| setupRecyclerView(rv); | ||
| rv = (RecyclerView) inflater.inflate(R.layout.pictures_list_fragment, container, false); | ||
|
|
||
| if (mProgressDialog != null && mProgressDialog.isShowing()) | ||
| mProgressDialog.dismiss(); | ||
|
|
||
| mProgressDialog = new ProgressDialog(container.getContext()); | ||
| mProgressDialog.setIndeterminate(true); | ||
| mProgressDialog.setMessage("Loading..."); | ||
| mProgressDialog.show(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All this code is messy. He should have at least encapsulated some parts in functions. |
||
|
|
||
| mBus = TinyBus.from(getActivity()); | ||
|
|
||
| if (mData == null) { | ||
| if (!loading) { | ||
| RestClient restClient = RestClient.getInstance(); | ||
| restClient.getPhotos(mBus); | ||
| loading = true; | ||
| } | ||
| } else { | ||
| drawList(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dialog is hidden in drawlist(), but if mData was != null, he shouldn't have ever shown it. I get more worried about this kind of thins. A medium-good programmer that moreover is trying to impress you... I want to think I wouldn't write such a spaghetti code for something relatively simple. |
||
| } | ||
| return rv; | ||
| } | ||
|
|
||
| private void setupRecyclerView(RecyclerView recyclerView) { | ||
| recyclerView.setLayoutManager(new LinearLayoutManager(recyclerView.getContext())); | ||
| recyclerView.setAdapter(new PhotosAdapter()); | ||
| @Override | ||
| public void onResume() { | ||
| super.onResume(); | ||
| mBus.register(this); | ||
| } | ||
|
|
||
| @Override | ||
| public void onPause() { | ||
| super.onPause(); | ||
| mBus.unregister(this); | ||
| } | ||
|
|
||
| @Subscribe | ||
| public void onAlbumEvent(AlbumEvent apiEvent) { | ||
|
|
||
| if (mProgressDialog.isShowing()) | ||
| mProgressDialog.dismiss(); | ||
| if (apiEvent.getError() != null) { | ||
| Crouton.makeText(getActivity(), apiEvent.getError().getMessage(), Style.ALERT).show(); | ||
| } else { | ||
| mData = apiEvent.getObject(); | ||
| drawList(); | ||
| } | ||
| loading = false; | ||
| } | ||
|
|
||
| public void drawList() { | ||
| if (mProgressDialog.isShowing()) | ||
| mProgressDialog.dismiss(); | ||
| rv.setLayoutManager(new LinearLayoutManager(rv.getContext())); | ||
| PhotosAdapter photosAdapter = new PhotosAdapter(); | ||
| photosAdapter.setmData(mData); | ||
| photosAdapter.setFragmentManager(getActivity().getSupportFragmentManager()); | ||
| rv.setAdapter(photosAdapter); | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| package android.coding.interview.makeitawesome.model; | ||
|
|
||
| /** | ||
| * Created by antonio on 24/07/15. | ||
| */ | ||
| public class Album { | ||
|
|
||
| private int albumId; | ||
| private int id; | ||
| private String title; | ||
| private String url; | ||
| private String thumbnailUrl; | ||
|
|
||
| public Album() { | ||
| this.albumId = -1; | ||
| this.id = -1; | ||
| this.title = ""; | ||
| this.url = ""; | ||
| this.thumbnailUrl = ""; | ||
| } | ||
|
|
||
| public Album(int albumId, int id, String title, String url, String thumbnailUrl) { | ||
| this.albumId = albumId; | ||
| this.id = id; | ||
| this.title = title; | ||
| this.url = url; | ||
| this.thumbnailUrl = thumbnailUrl; | ||
| } | ||
|
|
||
| public int getAlbumId() { | ||
| return albumId; | ||
| } | ||
|
|
||
| public void setAlbumId(int albumId) { | ||
| this.albumId = albumId; | ||
| } | ||
|
|
||
| public int getId() { | ||
| return id; | ||
| } | ||
|
|
||
| public void setId(int id) { | ||
| this.id = id; | ||
| } | ||
|
|
||
| public String getTitle() { | ||
| return title; | ||
| } | ||
|
|
||
| public void setTitle(String title) { | ||
| this.title = title; | ||
| } | ||
|
|
||
| public String getUrl() { | ||
| return url; | ||
| } | ||
|
|
||
| public void setUrl(String url) { | ||
| this.url = url; | ||
| } | ||
|
|
||
| public String getThumbnailUrl() { | ||
| return thumbnailUrl; | ||
| } | ||
|
|
||
| public void setThumbnailUrl(String thumbnailUrl) { | ||
| this.thumbnailUrl = thumbnailUrl; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keeping
imgreference here in callback causes multiple loading of different urls into same position on fast scrolling and then waiting for load.Easily fixed by just simple loading with
.into(holder.mImage)without callback.