-
Notifications
You must be signed in to change notification settings - Fork 135
Support efficient parsing of Request ResourceParameters #62
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
Comments
Thank you for the feedback. The behavior of the ResourceParameters class is indeed not optimal.
I think I would go and replace the existing function in that case due to the ambiguity that you've mentioned. That would lead us to something with a signature like // ...
std::string idVal;
if (req->getParams()->getRequestParameter("id", idVal) {
// handle request, idVal is set now
} else {
// handle missing parameter
}
// ...
I though about removing the So I'd derive the following tasks:
Does that make sense from your point of view? Then I'd put it into the next major release, as the changes may break existing code. |
Looks like a great good solution :-). Indeed since the get...Int function only converts unsigned 16bit integers there are likely more often cases where it will cause confusion than be helpful. (if all usecases for signed/unsigned 8,16,32bit are equally distributed) ;-) Btw: If I could trouble you to add one more feature while you are at it: Currently I might have to deal with a situation where I have to parse request parameters of the form "t10=123&t23=345,t40=456,choice=a" where the parameter name itself contains information. If so, I would have to guess and check for each possible parameter name. |
Getting the names should be possible by iterating through the parameters (the third task). The parameters are stored as Would that be sufficient for you? |
Giving access to an iterator to the string pairs would, i believe, be indeed the most straightforward way to cover all use-cases. |
Could you have a look at #65 and see if that works for you? I'll merge that with the other pending PRs to create a new release, but that may take some time. |
Is your feature request related to a problem? Please describe.
It seems that the recommend way to parse a requests RequestParameters, is to call
isRequestParameterSet
and thengetRequestParameter
for all the key-strings that might make up a query.Both of these Methods translate to a for-loop which iterates though the whole list of parameters, doing string compare on each step.
Unfortunately
getRequestParameter
in itself is insufficient to check for the existence of a parameter since it will return "" when a parameter is unset or set to empty.getRequestParameterInt
is even worse, since it will return 0 for unset parameters and thus one can't differentiate from parameters set to value 0.Describe the solution you'd like
In order to improve response timing, I'd like a method that combines
isRequestParameterSet
andgetRequestParameter
. e.g.getRequestParameterIfExists
which returns true/false and sets the parameter into a given argument.Describe alternatives you've considered
ESP32AsyncWebserver let's people iterate for the parameters and do string comparison themselves, which is better than iterating twice but worse that a
getRequestParameterIfExists
method, since such a loop can't abort early and not exclude parameters from checking that have already been found.The text was updated successfully, but these errors were encountered: