Skip to content
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

A couple of ideas #76

Open
superwills opened this issue Apr 10, 2024 · 2 comments
Open

A couple of ideas #76

superwills opened this issue Apr 10, 2024 · 2 comments

Comments

@superwills
Copy link

superwills commented Apr 10, 2024

Your library seems very promising, but I thought I would let you know a few ideas I had while looking at it:

  • Would be really great to get https support
  • I'd request to avoid the use of C++ exceptions. Many C++ devs don't use them very much (they're notoriously expensive!)
  • The single header, although it looks "easier to use" at first, is really difficult to browse. Splitting the source file into multiple files is almost always the best idea.
  • style nit: I feel like you're overusing auto in the code. I mean, in this case, int is shorter ;) The general guideline I've heard is to use auto where it "increases readability", very subjective :)
  • There is some extra data copy that could be avoided. Consider going to C-arrays/memcpy for passing around some of the data?
  • Would be nice to have an asyncSend that accepts a lambda onComplete handler (something like std::function<void (Response)>)
@superwills
Copy link
Author

See also this softwareengineering.stackexchange answer on exceptions being considered an antipattern

@Nevermore1994
Copy link

I rewrote the HTTP request.
https://github.com/Nevermore1994/http-request

In this repo,It sounds like all the ideas you mentioned have already been implemented.

By the way, auto is quite beneficial for C++ development. Using auto extensively does not increase the cognitive load for developers; on the contrary, it can significantly reduce the burden on both developers and the compiler.

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

No branches or pull requests

2 participants