Usage of new API, cleanups (reduced)

src/org/adblockplus/android/AdvancedPreferences.java

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 27 matching lines @@
 import android.text.TextUtils;
 import android.util.Log;
 import android.view.View;
 import android.widget.ScrollView;
 import android.widget.TextView;
 import android.widget.Toast;
 
 /**
  * Advanced settings UI.
  */
+@SuppressWarnings("deprecation")

[Felix Dahlke, 2014/04/28 07:29:01]

I don't think we should ignore this, in general pretty much. There are a bunch
of warnings anyway, and deprecation warnings can usually be fixed.

[René Jeschke, 2014/04/28 08:34:32]

Those deprecation warnings can only be resolved by raising the minimum API level
to 11 and refactoring the whole app to use the fragment API.

Still, removed (also this will introduce a lot of additional noise to any code
improvement tries as one has to filter out which warnings are serious and which
are not).

[Felix Dahlke, 2014/04/28 10:09:41]

It may make sense then, however, I think it belongs in a separate review then,
this one is pretty big already.

[René Jeschke, 2014/04/28 10:18:34]

Done.

 public class AdvancedPreferences extends SummarizedPreferences
 {
-  private static final String TAG = "AdvancedPreferences";
-
+  private static final String TAG = Utils.getTag(AdvancedPreferences.class);
   private static final int CONFIGURATION_DIALOG = 1;
 
   private ProxyService proxyService = null;
 
   @Override
-  public void onCreate(Bundle savedInstanceState)
+  public void onCreate(final Bundle savedInstanceState)
   {
     super.onCreate(savedInstanceState);
 
     addPreferencesFromResource(R.xml.preferences_advanced);
 
-    SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this);
+    final SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this);
 
-    PreferenceScreen screen = getPreferenceScreen();
+    final PreferenceScreen screen = getPreferenceScreen();
     if (ProxyService.NATIVE_PROXY_SUPPORTED)
     {
       screen.removePreference(findPreference(getString(R.string.pref_proxy)));
       if (prefs.getBoolean(getString(R.string.pref_proxyautoconfigured), false))
       {
         screen.removePreference(findPreference(getString(R.string.pref_proxyenabled)));
       }
     }
     if (getResources().getBoolean(R.bool.def_release))
     {
       screen.removePreference(findPreference(getString(R.string.pref_support)));
     }
     else
     {
-      Preference prefUpdate = findPreference(getString(R.string.pref_checkupdate));
+      final Preference prefUpdate = findPreference(getString(R.string.pref_checkupdate));
       prefUpdate.setOnPreferenceClickListener(new OnPreferenceClickListener()
       {
-        public boolean onPreferenceClick(Preference preference)
+        @Override
+        public boolean onPreferenceClick(final Preference preference)
         {
-          AdblockPlus application = AdblockPlus.getApplication();
+          final AdblockPlus application = AdblockPlus.getApplication();
           application.checkUpdates();
           return true;
         }
       });
 
-      Preference prefConfiguration = findPreference(getString(R.string.pref_configuration));
+      final Preference prefConfiguration = findPreference(getString(R.string.pref_configuration));
       prefConfiguration.setOnPreferenceClickListener(new OnPreferenceClickListener()
       {
-        public boolean onPreferenceClick(Preference preference)
+        @Override
+        public boolean onPreferenceClick(final Preference preference)
         {
           showDialog(CONFIGURATION_DIALOG);
           return true;
         }
       });
     }
   }
 
   @Override
   public void onResume()
   {
     super.onResume();
-    SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this);
-    int refresh = Integer.valueOf(prefs.getString(getString(R.string.pref_refresh), "0"));
+    final SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this);
+    final int refresh = Integer.valueOf(prefs.getString(getString(R.string.pref_refresh), "0"));
     findPreference(getString(R.string.pref_wifirefresh)).setEnabled(refresh > 0);
     connect();
   }
 
   @Override
   public void onPause()
   {
     super.onPause();
     disconnect();
   }
 
   @Override
-  public void onSharedPreferenceChanged(SharedPreferences sharedPreferences, String key)
+  public void onSharedPreferenceChanged(final SharedPreferences sharedPreferences, final String key)
   {
     if (getString(R.string.pref_proxyenabled).equals(key))
     {
-      AdblockPlus application = AdblockPlus.getApplication();
-      boolean enabled = sharedPreferences.getBoolean(key, false);
-      boolean serviceRunning = application.isServiceRunning();
+      final AdblockPlus application = AdblockPlus.getApplication();
+      final boolean enabled = sharedPreferences.getBoolean(key, false);
+      final boolean serviceRunning = application.isServiceRunning();
       if (enabled)
       {
         if (!serviceRunning)
           startService(new Intent(this, ProxyService.class));
       }
       else
       {
         if (serviceRunning)
           stopService(new Intent(this, ProxyService.class));
         // If disabled, disable filtering as well
-        SharedPreferences.Editor editor = sharedPreferences.edit();
+        final SharedPreferences.Editor editor = sharedPreferences.edit();
         editor.putBoolean(getString(R.string.pref_enabled), false);
         editor.commit();
         application.setFilteringEnabled(false);
       }
     }
     if (getString(R.string.pref_refresh).equals(key))
     {
-      int refresh = Integer.valueOf(sharedPreferences.getString(key, "0"));
+      final int refresh = Integer.valueOf(sharedPreferences.getString(key, "0"));
       findPreference(getString(R.string.pref_wifirefresh)).setEnabled(refresh > 0);
     }
     if (getString(R.string.pref_crashreport).equals(key))
     {
-      boolean report = sharedPreferences.getBoolean(key, getResources().getBoolean(R.bool.def_crashreport));
+      final boolean report = sharedPreferences.getBoolean(key, getResources().getBoolean(R.bool.def_crashreport));
       try
       {
-        CrashHandler handler = (CrashHandler) Thread.getDefaultUncaughtExceptionHandler();
+        final CrashHandler handler = (CrashHandler) Thread.getDefaultUncaughtExceptionHandler();
         handler.generateReport(report);
       }
-      catch (ClassCastException e)
+      catch (final ClassCastException e)
       {
         // ignore - default handler in use
       }
     }
     super.onSharedPreferenceChanged(sharedPreferences, key);
   }
 
   @Override
-  protected Dialog onCreateDialog(int id)
+  protected Dialog onCreateDialog(final int id)
   {
     Dialog dialog = null;
     switch (id)
     {
       case CONFIGURATION_DIALOG:
-        List<String> items = new ArrayList<String>();
+        final List<String> items = new ArrayList<String>();
         items.add(AdblockPlus.getDeviceName());
         items.add(String.format("API: %d Build: %d", Build.VERSION.SDK_INT, AdblockPlus.getApplication().getBuildNumber()));
         if (proxyService != null)
         {
           items.add(String.format("Local port: %d", proxyService.port));
           if (proxyService.isTransparent())
           {
             items.add("Running in root mode");
             items.add("iptables output:");
-            List<String> output = proxyService.getIptablesOutput();
+            final List<String> output = proxyService.getIptablesOutput();
             if (output != null)
             {
-              for (String line : output)
+              for (final String line : output)
               {
                 if (!"".equals(line))
                   items.add(line);
               }
             }
           }
           if (proxyService.isNativeProxyAutoConfigured())
           {
             items.add("Has native proxy auto configured");
           }
           if (ProxyService.NATIVE_PROXY_SUPPORTED)
           {
-            String[] px = ProxySettings.getUserProxy(getApplicationContext());
+            final String[] px = ProxySettings.getUserProxy(getApplicationContext());
             if (px != null)
             {
               items.add("System settings:");
               items.add(String.format("Host: [%s] Port: [%s] Excl: [%s]", px[0], px[1], px[2]));
             }
           }
           items.add("Proxy settings:");
-          items.add(String.format("Host: [%s] Port: [%s] Excl: [%s]", proxyService.proxy.props.getProperty("adblock.proxyHost"), proxyService.proxy.props.getProperty("adblock.proxyPort"),
+          items.add(String.format("Host: [%s] Port: [%s] Excl: [%s]", proxyService.proxy.props.getProperty("adblock.proxyHost"),

[Felix Dahlke, 2014/04/28 07:29:01]

Seems like this adds a lot of trailing whitespace, based on the diff. Shouldn't
be changed really.

[René Jeschke, 2014/04/28 08:34:32]

Nope, just the diff view gone mad.

+              proxyService.proxy.props.getProperty("adblock.proxyPort"),
               proxyService.proxy.props.getProperty("adblock.proxyExcl")));
           if (proxyService.proxy.props.getProperty("adblock.auth") != null)
             items.add("Auth: yes");
         }
         else
         {
           items.add("Service not running");
         }
 
-        ScrollView scrollPane = new ScrollView(this);
-        TextView messageText = new TextView(this);
+        final ScrollView scrollPane = new ScrollView(this);
+        final TextView messageText = new TextView(this);
         messageText.setPadding(12, 6, 12, 6);
         messageText.setText(TextUtils.join("\n", items));
         messageText.setOnClickListener(new View.OnClickListener()
         {
-
           @Override
-          public void onClick(View v)
+          public void onClick(final View v)
           {
-            ClipboardManager manager = (ClipboardManager) getSystemService(CLIPBOARD_SERVICE);
-            TextView showTextParam = (TextView) v;
+            final ClipboardManager manager = (ClipboardManager) getSystemService(CLIPBOARD_SERVICE);
+            final TextView showTextParam = (TextView) v;
             manager.setText(showTextParam.getText());
             Toast.makeText(v.getContext(), R.string.msg_clipboard, Toast.LENGTH_SHORT).show();
           }
         });
         scrollPane.addView(messageText);
 
-        AlertDialog.Builder builder = new AlertDialog.Builder(this);
+        final AlertDialog.Builder builder = new AlertDialog.Builder(this);
         builder.setView(scrollPane).setTitle(R.string.configuration_name).setIcon(android.R.drawable.ic_dialog_info).setCancelable(false)
             .setPositiveButton(R.string.ok, new DialogInterface.OnClickListener()
             {
-              public void onClick(DialogInterface dialog, int id)
+              @Override

[Felix Dahlke, 2014/04/28 07:29:01]

I remarked about this before, but IIRC, the situation with @Override was this:

1. Mandatory for overriding base class methods since Java 5
2. Mandatory for implementing abstract/interface methods since Java 6
3. Not allowed (!) for implementing interface methods in Java 5

OnClickListener.onClick is abstract, so this means this wouldn't work in Java 5,
right? Aren't we still on Java 5?

[René Jeschke, 2014/04/28 08:34:32]

The whole Adblock Plus Android gets checked/compiled agains Java6 compliance
level. This is hardcoded in the project properties, so we should be fine.

[Felix Dahlke, 2014/04/28 10:09:41]

Alright, thought it was Java 5 for some reason.

[René Jeschke, 2014/04/28 10:18:34]

Done.

+              public void onClick(final DialogInterface dialog, final int id)
               {
                 dialog.cancel();
               }
             });
         dialog = builder.create();
         break;
     }
     return dialog;
   }
 
   private void connect()
   {
     bindService(new Intent(this, ProxyService.class), proxyServiceConnection, 0);
   }
 
   private void disconnect()
   {
     unbindService(proxyServiceConnection);
     proxyService = null;
   }
 
-  private ServiceConnection proxyServiceConnection = new ServiceConnection()
+  private final ServiceConnection proxyServiceConnection = new ServiceConnection()
   {
-    public void onServiceConnected(ComponentName className, IBinder service)
+    @Override
+    public void onServiceConnected(final ComponentName className, final IBinder service)
     {
       proxyService = ((ProxyService.LocalBinder) service).getService();
       Log.d(TAG, "Proxy service connected");
     }
 
-    public void onServiceDisconnected(ComponentName className)
+    @Override
+    public void onServiceDisconnected(final ComponentName className)
     {
       proxyService = null;
       Log.d(TAG, "Proxy service disconnected");
     }
   };
 }