search

embeddedmz / ftpclient-cpp

C++ client for making FTP requests
MIT License
204 stars 65 forks source link

General improvements #7

Closed Finkman closed 4 years ago

Finkman commented 4 years ago

Some improvements as mentioned.

embeddedmz commented 4 years ago

Thank you for this second contribution. I will review it tonight.

embeddedmz commented 4 years ago

I think it's time to let clang-format handle the code formatting. Don't hesitate if you have a suggestion.

LibCURL global initialization/cleanup functions : There's a typo error: (CurlHandle::instnace -> CurlHandle::instance).

I see that you have taken advantage of C++11 thread-safe static variables initialization.

I don't know if you have already seen this issue https://github.com/embeddedmz/ftpclient-cpp/issues/4 but ultimately, I think it's better to let the user call curl_global_init(CURL_GLOBAL_ALL); and curl_global_cleanup(); through some static methods for example and only if it's needed !

What do you think ? I think it's wise to keep it stupid simple and give the user the control over global libcurl initialization as he might be using libraries that already handle that. The only issue is that these functions are not thread safe and curl_global_cleanup() must only be called at the end of the program. This kind of undefined behavior is very hard to find/fix.

Finkman commented 4 years ago

I think it's time to let clang-format handle the code formatting. Don't hesitate if you have a suggestion.

See #8 as proposal.

Finkman commented 4 years ago

I was searching for a way to ask libcurl it it is initialized and then optionally do it internally, but haven't found a way.

Since, you have #4 which addresses that topic, let's do it in that context and do one after another.