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

Improve HttpClient Usage #7

Closed
nikmd23 opened this issue Feb 22, 2017 · 3 comments
Closed

Improve HttpClient Usage #7

nikmd23 opened this issue Feb 22, 2017 · 3 comments
Assignees
Milestone

Comments

@nikmd23
Copy link

nikmd23 commented Feb 22, 2017

Hello!

I'm a big fan of this library, and I'm glad that there's a .NET solution for web push.

I was looking through the source code, and I noticed an easy to stumble into code smell in the SendNotification implementation: a new HttpClient is created for each notification sent.

There's a good article explaining why this is bad, and how to fix it.

coryjthompson pushed a commit that referenced this issue Feb 24, 2017
@coryjthompson
Copy link
Member

Thank you for the great feedback @nikmd23, I was unaware of this!

I'm now using a shared instance pf HttpClient within my class which doesn't "solve" the issue however it allows the user to hold a static instance to my class which resolves the issue.

Does this fix the issue for you?

I wasn't too keen to store an HttpClient object after my class was disposed just in case i need it again.

@coryjthompson coryjthompson self-assigned this Mar 6, 2017
@coryjthompson coryjthompson added this to the 1.0.9 milestone Mar 6, 2017
@nikmd23
Copy link
Author

nikmd23 commented Mar 6, 2017

Sounds like a great improvement @coryjthompson. Thanks!

PS - I'm in the middle of recording a Pluralsight course on WebPush. In it, I'm both using and recommending your library! It worked great and was just what I needed.

@coryjthompson
Copy link
Member

Wow! Thank you, that is awesome. Would you mind sending a link once its published?

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

No branches or pull requests

2 participants