Skip to content
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

Merge sync and async functions into one using SmartCoro #46

Closed
wants to merge 1 commit into from
Closed

Merge sync and async functions into one using SmartCoro #46

wants to merge 1 commit into from

Conversation

friendlyanon
Copy link

Hey, I would like to hear your opinion on this before I dive into #27
The reason why I merged sync and async functions is because the logic was duplicated, which can be error prone. I wanted to get this out of the way before attempting #27 because whatever gets added would have had to be duplicated as well.

@sindresorhus
Copy link
Owner

This is clever, but IMHO a bit too clever. You are trading less duplication for complication and making it less readable. I don’t think duplication is that bad, especially not when the code is almost the same (just added await keyword) and well tested.

@friendlyanon
Copy link
Author

friendlyanon commented Jul 30, 2019

I have a discord bot of a modest size that makes use of this generator wrapper (with additional features) and I find it greatly reduced complexity to allow a more declarative style of writing code. I can see this being overkill for this utility though.

Thank you for the response, I'll proceed accordingly.

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.

None yet

2 participants