Check if an image download from the Internet has already started

Question

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?

Answer 1

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).