Implement Stream API to KRisIO #144
Conversation
| @Throws(IOException::class) | ||
| public fun processToStream(reader: Reader): Stream<RisRecord> = runBlocking(Dispatchers.IO) { | ||
| val lineFlow = BufferedReader(reader).lineSequence().asFlow() | ||
| Stream.builder<RisRecord>().also { recordsStream -> |
There was a problem hiding this comment.
IMHO also semantically does not fit that well. I prefer to use apply in this case. Since I have a few other minor changes (small refactoring in the test, adding the new functionality to the guide, minor cleanup I noticed here but for stuff I introduced earlier), I will merge this PR as is and follow up with my own.
There was a problem hiding this comment.
Perfect, thanks! I'll try to keep your convention better next time
There was a problem hiding this comment.
You followed my conventions so well that you also copied my grammar mistakes from before :-) No, seriously, there's no complaint about you not following my conventions, as most changes in my follow up PR are fixing my own mistakes from before.
Only Regarding the scoping functions. To me the semantics of apply is about initialization or configurationof an object, whereas also uses the lambda to do something with the object after it has been used. To me apply is more fitting than also in our use case, as we prefill the builder before calling the build method. But I guess that's also opinionated.
Thanks for your contribution.
Changes:
When parsing on large RIS files (40k+ records) I've noticed large memory consumption. I think
KRisIOcould expose API with java stream to enable users to utilise stream on their own.