hashgraph / hedera-sdk-java

Hedera™ Hashgraph SDK for Java
https://docs.hedera.com/guides/docs/sdks
Apache License 2.0
231 stars 120 forks source link

Remove any use of streams in the SDK #735

Closed janaakhterov closed 2 years ago

janaakhterov commented 3 years ago

Description

We should not be using any stream API in the SDK because it is not compatible with Java 7 because the earliest stream support in Java came in Java 8. We don't have any tests that show this (and we really should), but at the moment our JDK 7 SDK releases are likely broken because we use streams a few places.

Steps to reproduce

Use SDK within a JDK 7 context

Additional context

No response

Hedera network

other

Version

v2.1.0

Operating system

Linux

steven-sheehy commented 3 years ago

I don't think this ticket is necessary. Streams are supported in Java 7 since you're not really using Java 8 streams, you're using a library which backports them. If you look closely you're not using java.util.stream but java8.util.stream from the streamsupport library.

streamsupport is a backport of the Java 8 java.util.function (functional interfaces) and java.util.stream (streams) API for Android and users of Java 6 or 7 supplemented with selected additions from java.util.concurrent which didn't exist back in Java 6.

You should definitely add a matrix build workflow for the JDK versions you support though to verify build correctness across JDKS.

mehcode commented 3 years ago

I asked @danielakhterov to open this as I'd much rather see us not use the back port library.

steven-sheehy commented 3 years ago

Except CompletableFuture is also provided by the streamsupport library and it is used everywhere in the code. Though admittedly it's in a different streamsupport dependency. Seems weird to trust one backport library but not another. But still fine if you want to remove it. I'm wondering when can we drop support for Java 7?

janaakhterov commented 3 years ago

From my understanding we want to eliminate any unnecessary dependencies to make our SDK code size smaller. We can implementing everything we want without using Java 8 streams, but we cannot get away with not using a CompletableFuture. As for when we can drop Java 7 support I believe is essentially never since we support JDK 7 because of Android. I know Android technically supports JDK 8, but from what I've seen that isn't completely true. IIRC there was an issue awhile ago where we were using HashMap.merge or HashMap.computeIfAbsent which is supported by JDK 8. However, we had people complaining it broke their application on Android. I do wish we can drop JDK 7 support as I'd be able to use all the new Java features, but I just don't think that'll be possible.

stefan-zobel commented 3 years ago

From my understanding we want to eliminate any unnecessary dependencies to make our SDK code size smaller.

For the record: it wouldn't be hard to refactor a standalone version of the streamsupport CompletableFuture library that doesn't depend on the Streams portion of the backport (I'm the author of the library).

janaakhterov commented 3 years ago

Heh, that might be a good idea for us then. Not the highest priority but decreasing our SDK size would be nice.

stefan-zobel commented 3 years ago

I am working on a minimized (105 KiB) version with a public API surface that only comprises the following interfaces / classes:

janaakhterov commented 3 years ago

That is awesome @stefan-zobel Thanks for taking the time to do that, much appreciated.

stefan-zobel commented 3 years ago

I've given it the name streamsupport-minifuture.

<dependency>
    <groupId>net.sourceforge.streamsupport</groupId>
    <artifactId>streamsupport-minifuture</artifactId>
    <version>1.7.4</version>
</dependency>

Hope this helps

janaakhterov commented 3 years ago

Thanks again @stefan-zobel We'll try to get this into our next sprint.

janaakhterov commented 2 years ago

Closing https://github.com/hashgraph/hedera-sdk-java/pull/782#issuecomment-1159231209