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 = Utils.getTag(AdvancedPreferences.class);
+
   private static final int CONFIGURATION_DIALOG = 1;
 
   private ProxyService proxyService = null;
 
   @Override
   public void onCreate(final Bundle savedInstanceState)
   {
     super.onCreate(savedInstanceState);
 
     addPreferencesFromResource(R.xml.preferences_advanced);
@@ skipping 136 matching lines @@
           if (ProxyService.NATIVE_PROXY_SUPPORTED)
           {
             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"),

[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"),
+          items.add(String.format("Host: [%s] Port: [%s] Excl: [%s]", proxyService.proxy.props.getProperty("adblock.proxyHost"), 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");
         }
 
         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(final View 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);
 
         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()
             {
               @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;
   }
@@ skipping 19 matching lines @@
     }
 
     @Override
     public void onServiceDisconnected(final ComponentName className)
     {
       proxyService = null;
       Log.d(TAG, "Proxy service disconnected");
     }
   };
 }