-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Source <> InputStream operations and chunks #134
Conversation
true | ||
} | ||
catch | ||
case t: Throwable => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NonFatal! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need a linter for that ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah no sorry 🤦 stream operators catch IE on purpose ... as these are propagated downstream. I discover this not the first time, I just need to document it somewhere visible ... ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This, and also I copied that from your code :) softwaremill/sttp#2158
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hehe yes, well, it just shows it's not that intuitive ;)
core/src/main/scala/ox/Chunk.scala
Outdated
/** An immutable finite indexed sequence of elements, backed by Array. Raw rrays are expensive performance-wise when you want to do | ||
* operations like concatenation, splitAt, drop, etc. Such operations are often useful when doing Source processing. `Chunk` offers a | ||
* wrapper focused primarly on performance optimizations for such costly operations, while maintaining familiar API of an `IndexedSeq`. | ||
* Underneath, a `Chunk` should leverage lazy data structures to avoid unnecessary data copying, which is a typical drawback for most Array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it does? it looks like a wrapper on an array, are there any optimizations in palce?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no optimizations yet, that's why I wrote that it "should" use these optimizations. I intended this PR to expose the Chunk API but start with simple non-optimized array operations. It can then later evolved as needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, sounds good, let's just explicitly write that: Chunk for now is a think wrapper on arrays, in the future we want to optimize its operations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from chunk - looks good :) I mean, chunk is ok as well, but I'm not sure if it's complete
Thanks :) Can you add follow-up tasks to (a) optimize the chunks, (b) add more input/output methods, I think reading/writing to/from |
Chunk task: #136
|
Yes, or just source.toOutputStream(OutputStream), same as you might have source.toFile(File) (or Path). We could also write a
I guess But let's open the tasks, we can discuss there further. |
Thanks :) Ok, we'll need to document this properly then. |
No description provided.