Skip to content

Conversation

cyan33
Copy link

@cyan33 cyan33 commented Mar 6, 2018

Hi @zertosh,

Don't know if this repo is still under maintainence. Anyway, I've noticed that it uses a, b, c, d, e as placeholders for the %s to be replaced. This could be easily improved by using the spread operator. It enhances the readability and is more safe in case the length of the placeholders is more than 5.

Would love to listen to your thought on this. 😄

@adamdicarlo
Copy link

I'd be concerned that the spread operator in the function signature would result in the argument spreading being executed on every call, not just failing ones. How about old-school use of arguments when the condition fails?

@Sceat
Copy link

Sceat commented May 8, 2020

@adamdicarlo i don't see how is that a problem

@adamdicarlo
Copy link

@Sceat Yeah, me neither, now (that was a while ago). At the time I was thinking it could be a performance issue. With eliminating these kinds of checks in production builds, it's no issue at all (maybe that wasn't standard when I commented? 🤔 ) ... even for debug builds I don't think it's nearly inefficient enough to be a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants