4
\$\begingroup\$

In the method getView() in my implementation of ArrayAdapter I have to download image from Internet using the image's url and set downloaded image to ImageView. I have to check in getView() whether downloading of an image from the given url is already started because of recycling views in TwoWayGridView and because of some bug there (immediately after creating a grid view the adapter's method getView() is called multiple times unnecessarily for the first item in grid view). To check that I use ImageView's tag.

@Override
public View getView(int position, View convertView, ViewGroup parent) {
    View itemView = (convertView != null) ? convertView : inflateItemView();
    ImageView imageView = (ImageView) itemView.findViewById(R.id.image);
    String url = getItem(position);
    setImageView(imageView, url);
    return itemView;
}

private void setImageView(ImageView imageView, String url) {
    if (url == null) {
        imageView.setImageDrawable(null);
    } else {
        downloadAndSetImage(imageView, url);
    }
}

private final Object LOCK_CHECKING_TAG_AND_URL = new Object();

private void downloadAndSetImage(final ImageView imageView, final String url) {
    synchronized (LOCK_CHECKING_TAG_AND_URL) {
        int tagId = R.id.tag_prevent_unnecessary_image_reset;
        String tag = (String) imageView.getTag(tagId);
        if (tag != null && tag.equals(url)) {
            // Image downloading from the given url for this ImageView is already started.
            // There is no need to start new downloading.
            return;
        }
        imageView.setTag(tagId, url);
    }
    imageView.setImageDrawable(null);
    // ImageManager is my utility class for downloading images in background
    imageManager.getImage(imageView, url, new ImageManager.Listener() {
        @Override
        public void imageReady(Drawable drawable) {
            imageView.setImageDrawable(drawable);
        }
    });
}

Later I changed the downloadAndSetImage() method:

private void downloadAndSetImage(final ImageView imageView, final String url) {
    if (!isNeedToDownload(imageView, url)) {
        return;
    }
    imageView.setImageDrawable(null);
    imageManager.getImage(imageView, url, new ImageManager.Listener() {
        @Override
        public void imageReady(Drawable drawable) {
            imageView.setImageDrawable(drawable);
        }
    });
}

private boolean isNeedToDownload(View imageView, String url) {
    synchronized (LOCK_CHECKING_TAG_AND_URL) {
        int tagId = R.id.tag_prevent_unnecessary_image_reset;
        String tag = (String) imageView.getTag(tagId);
        if (tag != null && tag.equals(url)) {
            return false;
        }
        imageView.setTag(tagId, url);
        return true;
    }
}

But I think that the name isNeedToDownload() is bad (or may be the method itself is bad) because this boolean checking method not only checks some condition but also set a tag to ImageView. I think that the name like a checkDownloadingNecessity() and return type

class CheckingResult {
    public boolean isDownloadingNecessary;
}

would be better, but it is too excessively. What do you think?

\$\endgroup\$
1
\$\begingroup\$

Just a note:

... and return type

class CheckingResult {
    public boolean isDownloadingNecessary;
}

Please don't. An encapsulated boolean goes really too far. Especially on Android as an empty class costs 500 bytes. The method is probably fine, the name could be improved to something less boolean-like. You could also use a ConcurrentSet like

private final Set<imageView> startedDownloads = Sets.newConcurrentHashSet();

private boolean registerDownload(View imageView, String url) {
    return startedDownloads.add(imageView);
}

This assumes Guava, but without it, you could use a ConcurrentHashMap with a dummy value instead. No idea what everything your LOCK_CHECKING_TAG_AND_URL gets used for, so my replacement may not be equivalent. I also removed your tagging in favor of placing the View itself in the set, which may be completely wrong. But why did you chose to use tags instead?

With whatever reason, this should be explained.

If it was because of memory, you could use a WeakReference<View> (with a delegating equals, which makes it too complicated).

\$\endgroup\$
  • \$\begingroup\$ If to use a collection then I should store pairs of ImageView and url. Because of recycling views in TwoWayGridView (the class is like a ListView) the same ImageView can be used several times as different items. So there could happen that downloading from one url is started, and while it not finished yet the ImageView is used as another item to display another image. I think that to use tags is a little easier. Sorry for my English \$\endgroup\$ – Leonid Semyonov Oct 2 '14 at 6:39
  • \$\begingroup\$ @LeonidSemyonov Your English is surely good enough as I understood it immediately. Maybe you could use the url alone? It makes no sense to download a single url twice, doesn't it? I really hate this android recycling stuff... \$\endgroup\$ – maaartinus Oct 2 '14 at 7:09
  • \$\begingroup\$ No, in ImageManager.getImage() images are caching so there will be no downloading from internet if image was downloaded. But I must check whether I called the downloadAndSetImage() method for the given ImageView and url because before calling it I set null image (i.e. empty) to ImageView (to show that the image is downloading, even if the image was cached it takes some time to read it from SD card). \$\endgroup\$ – Leonid Semyonov Oct 2 '14 at 7:30
  • \$\begingroup\$ @LeonidSemyonov Makes sense! \$\endgroup\$ – maaartinus Oct 2 '14 at 7:32
  • 1
    \$\begingroup\$ But there is a bug in TwoWayGridView - the first item's getView() method is called several times unnecessarily at the start. So there is blinks because of repeated settings of null image and then an actual image. To avoid these blinks I check whether I already called downloadAndSetImage() for the given url and ImageView \$\endgroup\$ – Leonid Semyonov Oct 2 '14 at 7:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.