Support efficient parsing of Request ResourceParameters

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 then getRequestParameter 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 and getRequestParameter. 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.

Thank you for the feedback. The behavior of the ResourceParameters class is indeed not optimal.

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.
[...] In order to improve response timing, I'd like a method that combines isRequestParameterSet and getRequestParameter. e.g. getRequestParameterIfExists which returns true/false and sets the parameter into a given argument.

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 bool getRequestParameter(std::string const param, std::string &val) and calls like this:

  // ...
  std::string idVal;
  if (req->getParams()->getRequestParameter("id", idVal) {
    // handle request, idVal is set now
  } else {
    // handle missing parameter
  }
  // ...

getRequestParameterInt is even worse, since it will return 0 for unset parameters and thus one can't differentiate from parameters set to value 0.

I though about removing the getSomethingInt functions for some time now, as they are quite specific and only helpful in some special cases. This is one argument more for finally doing that. std::stoi, std::stol etc. allow more control by the programmer and handling such errors.

So I'd derive the following tasks:

Remove the getSomethingInt functions
Modify the getRequestParameter to differentiate between an unset parameter and an empty value
Provide an interface for the user to iterate through the existing parameters (that would allow handling arbitrary, unknown parameters, too).

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:
It would be nice to know how many query parameters are available in total and if possible even get a list of their names.

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.

Btw: If I could trouble you to add one more feature while you are at it:
It would be nice to know how many query parameters are available in total and if possible even get a list of their names.

Getting the names should be possible by iterating through the parameters (the third task). The parameters are stored as std::pairs of std::strings at the moment. So for your example, you'd get an iterator that yields the pairs ("t10", "123"), ("t23", "345"), ("t40", "456") and ("choice", "a"). Adding something like ResourceParameters::requestParameterCount() shouldn't be a problem either.

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.

Modify interface to query parameters. Close #62. Close #63

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.

Navigation Menu

Repository navigation

Support efficient parsing of Request ResourceParameters #62

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Support efficient parsing of Request ResourceParameters #62

Description

Activity

fhessel commented on Dec 2, 2019

btittelbach commented on Dec 2, 2019

fhessel commented on Dec 2, 2019

btittelbach commented on Dec 3, 2019

fhessel commented on Dec 5, 2019

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions