Interface design of simple unified access to json, xml, yaml, ini, etc. #259
Conversation
common/stream.h
Outdated
| }; | ||
|
|
||
| // read until EOF | ||
| ReadAll readall(size_t min_buf = 1024, size_t max_buf = 1024 * 1024 * 1024); |
There was a problem hiding this comment.
is the param 'min_buf' necessary?
There was a problem hiding this comment.
in case that you want it start from, say, 1MB, to avoid some unnecessary realloc()
common/simple_dom.h
Outdated
| class RootNode; | ||
| struct NodeWrapper; | ||
|
|
||
| // the interface for internal implementations |
There was a problem hiding this comment.
Can it be concealed for internal use and not disclosed externally?
There was a problem hiding this comment.
No，because it's also the binary interface.
common/simple_dom.h
Outdated
| NodeWrapper get(size_t i) const { | ||
| IF_RET({_node->get(i)}); | ||
| } | ||
| NodeWrapper get(str key) const { |
There was a problem hiding this comment.
What is the result if the get operation fails?
There was a problem hiding this comment.
an NodeWrapper with nullptr pointer to node implementation
There was a problem hiding this comment.
This requires a clear explanation and instructions for use.
common/simple_dom.h
Outdated
| // even if they are equivalent in binary form. | ||
| Node* parse(char* text, size_t size, int flags); | ||
|
|
||
| inline Node* parse(IStream::ReadAll&& buf, int flags) { |
There was a problem hiding this comment.
Isn't the Node meant for internal use? How come it's being used by users here?
There was a problem hiding this comment.
parse(...)->wrap()
// 3. returning a pointer (of Node) is more efficient than an object (of Document),
// even if they are equivalent in binary form.
There was a problem hiding this comment.
For users, it's a direct use after parsing, and calling wrap is a burden.
There was a problem hiding this comment.
It a problem. The returned NodeImpl* is not the type used by the user and users must first determine if the returned NodeImpl* is nullptr before calling wrap. it's best not to expose these to users.
common/simple_dom.h
Outdated
| else return {}; | ||
|
|
||
| // the interface for users | ||
| struct NodeWrapper { |
There was a problem hiding this comment.
For user use, it's better to be named as "Node". The original "Node" can be renamed to NodeBase or something else, and not be exposed externally.
with examples and tests to show the usage