json: switch json() to readJson() #82

proppy · 2016-04-15T22:07:25Z

pass the response as a mutable buffer to get rid of the leaking strdup().
rename the function to make it more obvious it's only meant to be called once.
delete obsolete json_ field.
make response_ and other FirebaseCall member private.

ed7coyne · 2016-04-15T23:26:54Z

src/Firebase.cpp

  //the buffers.
  buffer_ = DynamicJsonBuffer();
-  return buffer_.parseObject(response());
+  // NOTE(proppy): this effectively void the response_ buffer.
+  return buffer_.parseObject(const_cast<char*>(response_.c_str()));


While efficient I think we should just make a mutable copy of the response for now to pass in. This seems super "premature optimization" hack.

maybe &response_[0] would be cleaner and get rid of the const cast. The issue with an intermediate copy is that JsonObject relies on the underlying string for the data storage, so we'd have to add a new member.

- pass the response as a mutable buffer to get rid of the leaking strdup(). - rename the function to make it more obvious it's only meant to be called once. - delete obsolete json_ field. - make response_ and other FirebaseCall member private.

proppy · 2016-10-04T23:32:59Z

obsolete

proppy assigned ed7coyne Apr 15, 2016

ed7coyne reviewed Apr 15, 2016
View reviewed changes

json: switch json() to readJson()

6942658

- pass the response as a mutable buffer to get rid of the leaking strdup(). - rename the function to make it more obvious it's only meant to be called once. - delete obsolete json_ field. - make response_ and other FirebaseCall member private.

proppy force-pushed the fix-json branch from 67523bd to 6942658 Compare April 16, 2016 09:25

proppy mentioned this pull request Apr 23, 2016

2 api surfaces proposal #95

Closed

proppy added the core label Sep 26, 2016

proppy closed this Oct 4, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

json: switch json() to readJson() #82

json: switch json() to readJson() #82

proppy commented Apr 15, 2016 •

edited

Loading

ed7coyne Apr 15, 2016

proppy Apr 27, 2016 •

edited

Loading

proppy commented Oct 4, 2016

json: switch json() to readJson() #82

json: switch json() to readJson() #82

Conversation

proppy commented Apr 15, 2016 • edited Loading

ed7coyne Apr 15, 2016

Choose a reason for hiding this comment

proppy Apr 27, 2016 • edited Loading

Choose a reason for hiding this comment

proppy commented Oct 4, 2016

proppy commented Apr 15, 2016 •

edited

Loading

proppy Apr 27, 2016 •

edited

Loading