perf: avoid useless create new empty object
All Submissions:
- [x] Have you followed the guidelines in our Contributing document?
- [x] Have you checked to ensure there aren't other open Pull Requests for the same update/change?
- [ ] Have you written unit tests?
- [ ] Have you written unit tests that cover the negative cases (i.e.: if bad data is submitted, does the library respond properly)?
- [ ] This PR is associated with an existing issue?
Closing issues
Put closes #XXXX (where XXXX is the issue number) in your comment to auto-close the issue that your PR fixes.
If this is a new feature submission:
- [ ] Has the issue had a maintainer respond to the issue and clarify that the feature is something that aligns with the goals and philosophy of the project?
Potential Problems With The Approach
Test plan
I think there is more then one places use this syntax like hapi & koa template service. Can you make it right in once?
done
https://github.com/lukeautry/tsoa/pull/1692#discussion_r1786227992 Can I have reason?
#1692 (comment) Can I have reason?
This version:
if (headers) {
Object.keys(headers).forEach((name: string) => {
response.set(name, headers[name]);
});
}
is more performant than:
Object.keys(headers || {}).forEach((name: string) => {
response.set(name, headers[name]);
});
for the following reasons:
Unnecessary Operations: In the second version, Object.keys(headers || {}) is always called, even when headers is null or undefined. If headers is falsy, || {} creates an empty object ({}), and then Object.keys processes it. This results in a redundant operation when headers is falsy, as there are no keys to iterate over.
In contrast, the first version avoids calling Object.keys altogether if headers is falsy, saving the overhead of creating an empty object and calling Object.keys.
Control Flow Simplicity: The first version has a clear conditional check (if (headers)), and only when headers is truthy does the code proceed to iterate through the keys. This makes it easier for the JavaScript engine to optimize the code because there's no need to run operations when they are unnecessary.
Reduced Memory Usage: In the second version, when headers is falsy, the code creates a new empty object ({}) that only serves to satisfy the Object.keys call. Although the memory impact of creating an empty object is small, it is still a waste when compared to the first version, which simply skips this step when not needed.
#1692 (comment) Can I have reason?
This version:
if (headers) { Object.keys(headers).forEach((name: string) => { response.set(name, headers[name]); }); }is more performant than:
Object.keys(headers || {}).forEach((name: string) => { response.set(name, headers[name]); });for the following reasons:
Unnecessary Operations: In the second version,
Object.keys(headers || {})is always called, even when headers is null or undefined. If headers is falsy, || {} creates an empty object ({}), and then Object.keys processes it. This results in a redundant operation when headers is falsy, as there are no keys to iterate over.In contrast, the first version avoids calling Object.keys altogether if headers is falsy, saving the overhead of creating an empty object and calling Object.keys.
Control Flow Simplicity: The first version has a clear conditional check (if (headers)), and only when headers is truthy does the code proceed to iterate through the keys. This makes it easier for the JavaScript engine to optimize the code because there's no need to run operations when they are unnecessary.
Reduced Memory Usage: In the second version, when headers is falsy, the code creates a new empty object (
{}) that only serves to satisfy the Object.keys call. Although the memory impact of creating an empty object is small, it is still a waste when compared to the first version, which simply skips this step when not needed.
logically, header should not include the undefined or null value.
It should be represents empty with {} empty object.
In this situation, examine its existence is a logically wrong. the why it design like version 1 is a historical reason, at the time of refactoring it I just simply have no time to think deeper but just trying to make it easier when reading, that's why I didn't wrap with anoterh if statement.
Since you raise the clean code issue, Let take the "more clean" solutions that we deal with the root cause: caller need to correctly pass the parameter without any incorrect type. If we do so, we can save a if statement, stay clean.
done