pistache
pistache copied to clipboard
Add set request attribute #1080
The issue #1080 has some time and I didn't saw pull requests, I think this is very useful, so I make some code that do it.
I created two commits:
- the code
- the ninja clang-format that the git commit scripts asks
Suggestions for more tests? Or names? Thanks!
Hi, I am not a contributor of Pistache, but I can provide the following feedback.
-
getData
,tryGetData
andgetDataAs
: I would personally condense these functions into one single function returning astd::optional
. The optional should be parametrized with the type of data I want it to hold (just like whatgetDataAs
currently does). - I prefer
setData
instead ofputData
: as a user, knowing that there is aget
function, I would look for aset
corresponding one rather than aput
one. - I am not 100% sure that
removeData
should throw an error if the key does not exist. - In my opinion, the naming of the functions can be improved to reflect their intent. What we want to do is to set and get the attributes of a request:
setAttribute
andgetAttribute
are intuitive to me, whereas I would have more trouble understanding the intent ofsetData
andgetData
(what does "data" refer to? is it perhaps the HTTP body?).
Hi. Thank you! I think your observations are good, I was trying to make similar to the Pistache::Tcp::Peer code https://github.com/pistacheio/pistache/blob/master/include/pistache/peer.h#L54
I am not sure how to proceed is this case.