[android] Fixed review notes

android/src/com/mapswithme/maps/ads/Banner.java

@@ -86,17 +86,25 @@ public final class Banner implements Parcelable
   @Override
   public boolean equals(Object o)
   {
-    if (this == o) return true;
-    if (o == null || getClass() != o.getClass()) return false;
+    if (this == o)
+      return true;
+
+    if (o == null || getClass() != o.getClass())
+      return false;
 
     Banner banner = (Banner) o;
 
+    if (mType != banner.mType)
+      return false;
+
     return mId.equals(banner.mId);
   }
 
   @Override
   public int hashCode()
   {
-    return mId.hashCode();
+    int result = mId.hashCode();
+    result = 31 * result + mType;
+    return result;
   }
 }

android/src/com/mapswithme/maps/ads/CachingNativeAdLoader.java

@@ -58,8 +58,7 @@ abstract class CachingNativeAdLoader extends BaseNativeAdLoader
       return;
     }
 
-    if (mTracker != null && mTracker.isImpressionGood(cachedAd.getProvider(), cachedAd.getBannerId())
-        && SystemClock.elapsedRealtime() - cachedAd.getLoadedTime() >= REQUEST_INTERVAL_MS)
+    if (isImpressionGood(cachedAd) && canBeReloaded(cachedAd))
     {
       LOGGER.d(TAG, "A new ad will be loaded because the previous one has a good impression");
       loadAdInternally(context, bannerId);
@@ -72,6 +71,16 @@ abstract class CachingNativeAdLoader extends BaseNativeAdLoader
     }
   }
 
+  private boolean isImpressionGood(@NonNull CachedMwmNativeAd ad)
+  {
+    return mTracker != null && mTracker.isImpressionGood(ad.getProvider(), ad.getBannerId());
+  }
+
+  private static boolean canBeReloaded(@NonNull CachedMwmNativeAd ad)
+  {
+    return SystemClock.elapsedRealtime() - ad.getLoadedTime() >= REQUEST_INTERVAL_MS;
+  }
+
   private void loadAdInternally(@NonNull Context context, @NonNull String bannerId)
   {
     if (isAdLoading(bannerId))

android/src/com/mapswithme/maps/ads/CompoundNativeAdLoader.java

@@ -9,6 +9,7 @@ import com.mapswithme.util.concurrency.UiThread;
 import com.mapswithme.util.log.Logger;
 import com.mapswithme.util.log.LoggerFactory;
 
+import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -28,6 +29,8 @@ public class CompoundNativeAdLoader extends BaseNativeAdLoader implements Native
   private static final Logger LOGGER = LoggerFactory.INSTANCE.getLogger(LoggerFactory.Type.MISC);
   private static final String TAG = CompoundNativeAdLoader.class.getSimpleName();
   private static final int TIMEOUT_MS = 5000;
+  @NonNull
+  private final List<NativeAdLoader> mLoaders = new ArrayList<>();
   @Nullable
   private final OnAdCacheModifiedListener mCacheListener;
   @Nullable
@@ -36,7 +39,6 @@ public class CompoundNativeAdLoader extends BaseNativeAdLoader implements Native
   private final Set<String> mFailedProviders = new HashSet<>();
   @Nullable
   private Runnable mDelayedNotification;
-  private int mBannerCount;
   /**
    * Indicates about whether the composite loading can be considered as completed or not.
    */
@@ -53,21 +55,22 @@ public class CompoundNativeAdLoader extends BaseNativeAdLoader implements Native
   public void loadAd(@NonNull Context context, @NonNull List<Banner> banners)
   {
     LOGGER.i(TAG, "Load ads for " + banners);
+    cancelLoaders();
     mLoadingCompleted = false;
-    mBannerCount = banners.size();
+    mFailedProviders.clear();
 
-    if (mBannerCount == 0)
+    if (banners.size() == 0)
       return;
 
     for (Banner banner : banners)
     {
-      NativeAdLoader loader = Factory.createLoaderForBanner(banner, mCacheListener, mAdTracker);
-      loader.setAdListener(this);
-
       if (TextUtils.isEmpty(banner.getId()))
         throw new AssertionError("A banner id mustn't be empty!");
 
+      NativeAdLoader loader = Factory.createLoaderForBanner(banner, mCacheListener, mAdTracker);
+      loader.setAdListener(this);
       loader.loadAd(context, banner.getId());
+      mLoaders.add(loader);
     }
   }
 
@@ -101,7 +104,7 @@ public class CompoundNativeAdLoader extends BaseNativeAdLoader implements Native
       return;
 
     // If only one banner is requested and obtained, it will be posted immediately to the listener.
-    if (mBannerCount == 1)
+    if (mLoaders.size() == 1)
     {
       onAdLoadingCompleted(ad);
       return;
@@ -135,7 +138,7 @@ public class CompoundNativeAdLoader extends BaseNativeAdLoader implements Native
     mFailedProviders.add(ad.getProvider());
 
     // If all providers give nothing, the listener will be notified about the error.
-    if (mFailedProviders.size() == mBannerCount)
+    if (mFailedProviders.size() == mLoaders.size())
     {
       if (getAdListener() != null)
         getAdListener().onError(ad, error);
@@ -167,6 +170,13 @@ public class CompoundNativeAdLoader extends BaseNativeAdLoader implements Native
     mLoadingCompleted = true;
   }
 
+  private void cancelLoaders()
+  {
+    for (NativeAdLoader adLoader : mLoaders)
+      adLoader.setAdListener(null);
+    mLoaders.clear();
+  }
+
   private class DelayedNotification implements Runnable
   {
     @NonNull

android/src/com/mapswithme/maps/ads/MyTargetAdsLoader.java

@@ -45,7 +45,7 @@ class MyTargetAdsLoader extends CachingNativeAdLoader implements NativeAd.Native
   public void onNoAd(String s, NativeAd nativeAd)
   {
     LOGGER.w(TAG, "onNoAd s = " + s);
-    CachedMwmNativeAd ad = new MyTargetNativeAd(nativeAd, 0);
+    CachedMwmNativeAd ad = new MyTargetNativeAd(nativeAd, 0 /* timestamp */);
     onError(ad.getBannerId(), ad, new MyTargetAdError(s));
   }
 

android/src/com/mapswithme/maps/ads/NativeAdLoader.java

@@ -10,7 +10,7 @@ public interface NativeAdLoader
    * Loads an ad for the specified banner id. A caller will be notified about loading through
    * {@link NativeAdListener} interface.
    *
-   * @param context  An activity context.
+   * @param context An activity context.
    * @param bannerId A banner id that ad will be loaded for.
    */
   void loadAd(@NonNull Context context, @NonNull String bannerId);

android/src/com/mapswithme/maps/bookmarks/data/MapObject.java

@@ -84,7 +84,6 @@ public class MapObject implements Parcelable
   private Banner[] readBanners(Parcel source)
   {
     Parcelable[] parcelables = source.readParcelableArray(Banner.class.getClassLoader());
-    Banner[] banners = null;
     if (parcelables != null)
       return Arrays.copyOf(parcelables, parcelables.length, Banner[].class);
     return null;