Skip to content
This repository has been archived by the owner on Apr 19, 2020. It is now read-only.

non-multi-threaded mode #15

Open
nevion opened this issue Mar 5, 2020 · 5 comments
Open

non-multi-threaded mode #15

nevion opened this issue Mar 5, 2020 · 5 comments

Comments

@nevion
Copy link

nevion commented Mar 5, 2020

I would prefer to default to threads off for simplicity and error propagation, but I think csv looks like a good project. Looking around at a few of the sources, it seems that this might not be too hard to make happen?

@Tehada
Copy link

Tehada commented Apr 16, 2020

I am 100% for default case when we don't spawn a thread. I used this lib for 2 days and already got 2 problems bcoz it uses thread internally:

  1. If i didn't close writer explicitly program just left hanging. Took some time to find out that this was thread of writer.
  2. If i close a writer (which is the only method of graceful shutdown available now) and right after this exit from program then sometimes writer just didn't write anything to my file.

The second problem is lack of proper api to wait until writer is actually done. But why solve it when you just can get rid of internal usage of threads which will solve a lot of problems which humanity already encountered while working with threads? Instead just make a wrapper, which spawns thread with writer, if you need them so much. Spawning threads internally is poor design, leading to unexpected behaviour and unnecessary complexity.

@Tehada
Copy link

Tehada commented Apr 16, 2020

Ok, I feel like my previous comment is not enough. Imagine Bob is working on big project (without csv support, yes) with custom threads and he really needs to use some csv library from github. Bob already have standartized approach for working with threads in his big project. Usually he spawns necessary threads during program startup and pass them approptiate functions to start with. Now he wants to use the single c++ csv library on github which has both reader and writer (I didn't spend too much time searching, but may be there is sth else). What he sees? This library already uses threads. And it uses std::thread, not Bob's project custom threads.

Ok, I hope this is enough to convince @p-ranav that users should have freedom of choosing whether to use threads or not, and which threads particulary they want to use.

Upd: I am willing to make pull request which will satisfy all sides.

Upd2: To make clear: I am not against using threads or other concurrent features in this project, I just want to split writer and thread logic so library will be far more flexible to use.

@nevion
Copy link
Author

nevion commented Apr 16, 2020

if you need gas to go - go. If it's simple and allows interests to be served, @p-ranav might not have any problems merging in time. I encountered bugs that other users posted as it was so it's technically a bug fix to me to switch to non-threaded.

@p-ranav
Copy link
Owner

p-ranav commented Apr 16, 2020

Feel free to create pull requests guys. I'll be more than happy to merge PRs that fix bugs or improve the design. Work's been crazy for me the last few months and I haven't found the time to address the many issues in this implementation.

@p-ranav
Copy link
Owner

p-ranav commented Apr 19, 2020

Hello,

I'm working on a single-threaded implementation of this library: https://github.com/p-ranav/csv2. The reader is ready for use. Check it out. Hopefully it works better. I'm planning to archive this repo in favor of csv2.

Sorry again for all the issues you've faced with this library.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants