Issue 4260 - Replace Apache Commons Lang classes

src/org/adblockplus/android/StringUtils.java

Index: src/org/adblockplus/android/StringUtils.java
diff --git a/src/org/adblockplus/android/StringUtils.java b/src/org/adblockplus/android/StringUtils.java
index be7c0af193d9c371c4ede9ea9c5847db44e57761..3af67b321b1fa25445c8055c4ae133219fb574cf 100644
--- a/src/org/adblockplus/android/StringUtils.java
+++ b/src/org/adblockplus/android/StringUtils.java
@@ -19,38 +19,38 @@ package org.adblockplus.android;
 
 public class StringUtils

[Felix Dahlke, 2016/09/12 15:53:28]

Might be worth a doc comment that this is supposed to be a drop-in replacement
for Apache Commons' StringUtils? Wouldn't insist on it though.

(On another note: I double checked that our code actually differs from Aparche
Commons' StringUtils, which it does. if it wouldn't, it would be alright as
well, but we should then point out in a comment that we are using some code from
that project here.)

[anton, 2016/09/13 06:27:14]

Actually this class has exactly the same methods signatures as original Apache
Commons in order not to change anything except package. But it's not
general-purpose replacement and it will not be used outside of the project (most
likely), so i don't think we need javadoc here. Also methods names are pretty
self-described.. Let me know if i should it anyway.
We were using Apache Commons and i'm not sure we've added that we are using it.
However this task is for replacing of apache commons classes so no Apache code
is used. For new methods impls we're not using any Apache code - i've took the
tests in original apache commons javadocs and wrote the code to pass the tests.

[Felix Dahlke, 2016/09/13 08:23:29]

Fair enough, let's leave it as is.
I was just pointing out that this is exactly the right approach :) 

[anton, 2016/09/13 10:39:04]

Acknowledged.

 {
-    public static boolean isNotEmpty(String value)
-    {
-        return value != null && value.length() > 0;
-    }
+  public static boolean isNotEmpty(String value)
+  {
+    return value != null && value.length() > 0;
+  }
+
+  public static boolean isEmpty(String value)
+  {
+    return !isNotEmpty(value);
+  }
 
-    public static boolean isEmpty(String value)
+  public static String join(Object[] array, String separator)
+  {
+    if (array == null)
     {
-        return !isNotEmpty(value);
+      return null;
     }
 
-    public static String join(Object[] array, String separator)
+    StringBuilder sb = new StringBuilder();
+    for (int i = 0; i < array.length; i++)
     {
-        if (array == null)
-        {
-            return null;
-        }
+      String eachValue = array[i].toString();
+      if (i > 0 && separator != null)
+      {
+        sb.append(separator);
+      }
 
-        StringBuilder sb = new StringBuilder();
-        for (int i = 0; i < array.length; i++)
-        {
-            String eachValue = array[i].toString();
-            if (i > 0 && separator != null)
-            {
-                sb.append(separator);
-            }
-
-            if (eachValue != null)
-            { 
-                sb.append(eachValue);
-            }
-        }
-
-        return sb.toString();
+      if (eachValue != null)

[Felix Dahlke, 2016/09/12 15:53:28]

So if the value is null, we'd just print the separator? Is that the intended
behaviour? (I suppose a few tests wouldn't hurt.)

[anton, 2016/09/13 06:27:14]

There is a difference between null string and empty ("") string.

For null i believe it's occasionally added and i believe we should just avoid
it. F.e. for 3 items array {"1",null,"2"} we should think it's only 2 items
{"1","2"} and only 1 separator should be added.

But empty string is full-featured string and we can't avoid it and 2 separators
should be added for {"1", "", "3"}.

To be discussed though.

[Felix Dahlke, 2016/09/13 08:23:29]

Makes sense, just wanted to make sure that's the intended behaviour. I looked at
the Apache Commons code for this and it does behave the same way, so that's
fine.

[anton, 2016/09/13 10:39:04]

Acknowledged.

+      { 
+        sb.append(eachValue);
+      }
     }
+
+    return sb.toString();
+  }
 }