Teammates: StringHelper: Use functional programming to improve code readability

[Zhiyuan-Amos]

Many methods in StringHelper use loops. They can be simplified to use Java 8 Streams instead.

[wkurniawan07]

Before proceeding any further, do realize that we're still in Java 7.

[Zhiyuan-Amos]

Yup, however I saw that there's a branch titled "upgrade to jdk8". I suppose the upgrade will be done sometime soon?

[wkurniawan07]

• The branch you saw was upgrading to JDK 8 with 1.7 compliance. It would be good if you did a check first; we have the issue #7609 for that.
• Java 8 support in GAE is still beta and there is no telling when it will become GA. Upgrading to Java 8 won't be done until at least 2-3 weeks after, so do not get too hyped up yet.

[Zhiyuan-Amos]

Ah I see, apologies for that!

[wkurniawan07]

This is a legitimate issue, it needs not be closed. However, it calls for an appropriate label for now.

[darrenwee]

I'll start working on this one.

[darrenwee]

I ran some benchmark tests on streams versus for-loops and found that streams generally perform worse.

I ran the tests using jmh.
The method names containing Vanilla are for the current methods in the teammates code base. I only did this for a few functions because it takes a while to write the new methods and run the benchmarks.
The metric used for performance is throughput which is how many times you can run the method per unit time (higher is better).

I don't think it's worth converting stuff over to streams unless the readability is really desired over performance, and even then people not used to the functional paradigm may not find it any improvement. I can still make some code quality changes but retain the current implementations.

@wkurniawan07

generateStringOfLength(int length, char character)

which basically duplicates character, length times.

Benchmark                             Mode  Cnt    Score   Error   Units
------------------------------------------------------------------------
generateStringOfLengthVanilla        thrpt   20   24.401 ± 0.243  ops/ms
generateStringOfLengthArrayFill      thrpt   20  164.210 ± 1.745  ops/ms
generateStringOfLengthConcatenation  thrpt   20    0.055 ± 0.001  ops/ms
generateStringOfLengthStreams        thrpt   20    4.530 ± 0.073  ops/ms
generateStringOfLengthStringJoin     thrpt   20    4.400 ± 0.204  ops/ms

toString(List<T> list, String delimiter)

which joins a list of elements together using delimiter.

Benchmark                             Mode  Cnt    Score   Error   Units
------------------------------------------------------------------------
toStringVanilla                      thrpt   20   25.682 ± 0.231  ops/ms
toStringStreams                      thrpt   20   22.671 ± 0.189  ops/ms

countEmptyStrings(String... strings)

which counts the number of strings which are null or empty in strings.

Benchmark                  Mode  Cnt    Score   Error   Units
------------------------------------------------------------------------
countEmptyStringsVanilla  thrpt   20  821.225 ± 3.186  ops/ms
countEmptyStringsStreams  thrpt   20  385.611 ± 2.571  ops/ms

(see gist here for implementation of each method)

[LiHaoTan]

@darrenwee thanks for your input. However, unless something is a bottleneck, this is premature optimization. Furthermore, this is probably naive benchmarking because there are cases where streams can definitely perform better. In addition, the JIT does a lot of optimizations when compiling the code and we are not accounting for that scenario here.

There are various discussions on the performance of streams, I would just link [1] and [2]. You can google for more if you are interested.

There are also many discussions on optimizations [1], which you might also want to take a look.

With that said, I don't disagree that if streams performance are really much worse (which it isn't), then, in that case, we might want to re-evaluate. If we use our intuition as well we would also find that it would not make sense for the Java 8 architects to design solutions that are not worth using.

unless the readability is really desired over performance, and even then people not used to the functional paradigm may not find it any improvement.

I agree with your point on readability, but I think by now we should have probably concluded that readability is usually more desirable than performance. The problem lies with the readability of Java 8's functional features, which of course if not used well can cause readability problems.

However, if we write good functional code, thanks to the expressivity and conciseness the code can become much more readable. The flip side is that it makes it very easy to write short and unreadable code. So while it isn't a silver bullet, there are many cases where it offers better readability that @wkurniawan07 has opened.

[darrenwee]

Okay, I'll proceed with the work then.