Skip to content
This repository was archived by the owner on Mar 17, 2025. It is now read-only.

std::unique_ptr should not be part of the Arduino facing class #93

Closed
proppy opened this issue Apr 22, 2016 · 6 comments
Closed

std::unique_ptr should not be part of the Arduino facing class #93

proppy opened this issue Apr 22, 2016 · 6 comments
Assignees

Comments

@proppy
Copy link
Contributor

proppy commented Apr 22, 2016

Follow up #90

Since it's unusual for arduino developer to use the stl.

@ed7coyne
Copy link
Collaborator

ed7coyne commented Apr 22, 2016

So I would propose leaving the {get,set,..}Ptr() calls as is but change the return type to be a standard pointer and include comments on all of them that the caller is responsible for the memory.

As a client I see value in having these calls as well, it is the natural c/c++ way to implement things like:

Firebase fbase(...);
FirebaseStream* stream;

void setup() {
  stream = fbase.getStreamPtr(...);
}

void loop() {
   event = stream->event();
}

@proppy
Copy link
Contributor Author

proppy commented Apr 22, 2016

I'm saying this mainly because of what I read on:
https://www.arduino.cc/en/Reference/APIStyleGuide

Don’t assume knowledge of pointers.
Beginning users of C find this the biggest roadblock, and get very confused by & and *,
so whenever you can avoid having them hanging out in the API, do so.

Arduino folks might also not be familiar with using -> instead of .

@proppy
Copy link
Contributor Author

proppy commented Apr 22, 2016

Maybe we need to find something simpler for Streaming like you suggested before since the lifecycle is different from regular other request/response interaction.

Something like.

Firebase fbase;
FirebaseStream stream;

void setup() {
  stream.begin(fbase);
}

void loop() {
  event = stream.event();
}

@ed7coyne
Copy link
Collaborator

Ya I think this may work into the larger issue of redesigning the API like
we are talking about for the transport implementation.
Like right now Firebase is a FirebaseCall factory. Do we really need it?
Or should you build calls directly and feed them to something else.

On Fri, Apr 22, 2016, 4:47 PM Johan Euphrosine notifications@github.com
wrote:

Maybe we need to find something simpler for Streaming like you suggested
before since the lifecycle is different from regular other request/response
interaction.


You are receiving this because you were assigned.
Reply to this email directly or view it on GitHub
#93 (comment)

@proppy
Copy link
Contributor Author

proppy commented Apr 23, 2016

Or should you build calls directly and feed them to something else.

Yes, that's pretty much the conclusion I came to with the transport impl:
https://github.com/proppy/firebase-arduino/blob/curl/openssl/main.cpp

So maybe we should put that on the backburner until I submit the PR for the transport stuff.

@proppy
Copy link
Contributor Author

proppy commented Apr 29, 2016

Closing this now that we have the arduino wrapper.

@proppy proppy closed this as completed Apr 29, 2016
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

2 participants