- Notifications
You must be signed in to change notification settings - Fork13.3k
Pass String by const reference [3.0]#6583
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
81a3e2f to28c745cCompare
earlephilhower left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
These are all basically private inside ESP8266WebServer, so I don't see any issue w/user classes even though they are virtual methods.
devyte commentedOct 3, 2019
The reason they're virtual is to allow a user to implement his own RequestHandler objects. |
earlephilhower commentedOct 3, 2019
Actually,@devyte, while what you say ispossible, IMO they're virtual because we need to store a pointer to a generic RequestHandler in the webserver (i.e. internal considerations, not custom overloads). Overloaded classes would suffer object slicing if we stored a pointer to the base class in the WebServer object, so these functions need to be virtual (even though I was able to rewrite the WebServer class to be a template w/o virtuals). |
Passing String by value means a full copy-constructor/temporarystring creation which is slightly inefficient. Avoid thatby using const references.
28c745c to561b7f4Compare| boolhandle(WebServerType& server, HTTPMethod requestMethod, String requestUri)override { | ||
| boolhandle(WebServerType& server, HTTPMethod requestMethod,const String& __requestUri)override { | ||
| StringrequestUri(__requestUri); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This String copy can be handled differently in a further fixing commit.
Passing String by value means a full copy-constructor/temporary
string creation which is slightly inefficient. Avoid that
by using const references.