-
Notifications
You must be signed in to change notification settings - Fork 41
Add .appendAll() function #35
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
base: master
Are you sure you want to change the base?
Conversation
Another option here would be to include this functionality in the |
Codecov Report
@@ Coverage Diff @@
## master #35 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 1 1
Lines 28 32 +4
=====================================
+ Hits 28 32 +4
Continue to review full report at Codecov.
|
Isn't this essentially toAppend.reduce((prev, curr) => prev.append(delim).append(curr), stmt) |
Yeah that does the same thing but it's more difficult to read and prevents you from being able to chain additional What do you think? |
Did you actually measure a performance difference? I doubt it has one (after all, you are IO-bound anyway for database queries). |
Just wanted to chime in on this ancient PR. I agree shorthand functions can sometimes make readability worse, but in this case I think the pattern in question is so commonplace that the cost of having to read a new custom method and "guess" its meaning is less than the cost of having to parse this pattern out on every occurrence. |
It seems to me that in most cases you want the
|
@felixfbecker Hi there. I don't know if everything has been said on this topic. But I agree on the stated positions that an appendAll would be helpful. |
Adds an .appendAll() function that allows you to pass an array of statements or strings to append. You can also provide a custom delimiter to separate the statements/strings (defaults to a single space)
Also updates Typescript typings, and README file with documentation.
Includes several unit tests.
Closes #32