upta / pubsub

An extremely light-weight, easy to use PCL pub/sub library
Apache License 2.0
219 stars 53 forks source link

Add SubscribeTask, PublishAsync #20

Closed TomaszStanisz closed 5 years ago

upta commented 5 years ago

Curious, what would the benefits be over simply switching it to be Task-based like I did in my async-test branch?

https://github.com/upta/pubsub/tree/async-test

I originally went down the path of doing separate [Method]Async calls, but since the whole goal of the library is to keep things simple I was wary of doing so and didn't see a benefit.

I could be wrong and there is one, of course :) I'm just not seeing it.

TomaszStanisz commented 5 years ago

Hi Brian,

Sorry for delay but I was busy and later I just forgot to answer 😊 I also didn’t check you second brunch before my changes (now I checked), but I still think that subscription with Task option and Publish which return Task can be useful.

The reasons why I add this task base methods is because:

  1. I already have methods which returns tasks and I want to subscribe with this methods. I know that I can create another void method and call Task.Run inside but it is more comfortable to have dedicated method.
  2. When you have both versions for publish (which return void or Task) and both version for subscribe (action and function with Task) then you can use it in all scenarios which you need:

    • Publish and do not wait for any subscriber (doesn’t matter if it is subscribed with action or task). You can just call PublishAsync without await
    • Publish and wait for all subscribers (doesn’t matter if it is subscribed with action or task). You can just call await PublishAsync.
    • Publish and wait for all subscribers subscribed with actions and don’t wait for subscribers subscribed with tasks. You can just call Publish.
    • Subscribe with action or with Task (depends from your needs) Br Tomasz

Od: Brian notifications@github.com Wysłano: 26 March 2019 22:04 Do: upta/pubsub pubsub@noreply.github.com DW: Tomasz Stanisz tomasz.stanisz@novacura.com; Author author@noreply.github.com Temat: Re: [upta/pubsub] Add SubscribeTask, PublishAsync (#20)

Curious, what would the benefits be over simply switching it to be Task-based like I did in my async-test branch?

https://github.com/upta/pubsub/tree/async-test

I originally went down the path of doing separate [Method]Async calls, but since the whole goal of the library is to keep things simple I was wary of doing so and didn't see a benefit.

I could be wrong and there is one, of course :) I'm just not seeing it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/upta/pubsub/pull/20#issuecomment-476851893, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AMerD0Xz7HzMwcg9b7Ic0JkPn60x-_2Jks5vaotRgaJpZM4btZj0.

fabricioferreira commented 5 years ago

@upta I was about to implement this wrapper myself when I saw this PR. Are there any plans to merge it and to make it official?

Thanks!

upta commented 5 years ago

@TomaszStanisz @fabricioferreira I'll be honest, I completely forgot about this. My apologies!

Looking back over it I'm fine with it with the changes. The only minor thing I think would be better would be to change the SubscribeTask methods to simply be called Subscribe; no reason they can't simply be an overload of the method and cleans up the public-facing surface a bit.

TomaszStanisz commented 5 years ago

No problem for me, I changed method name

From: Brian notifications@github.com Sent: Friday, July 19, 2019 4:48 PM To: upta/pubsub pubsub@noreply.github.com Cc: Tomasz Stanisz tomasz.stanisz@novacura.com; Mention mention@noreply.github.com Subject: Re: [upta/pubsub] Add SubscribeTask, PublishAsync (#20)

@TomaszStaniszhttps://github.com/TomaszStanisz @fabricioferreirahttps://github.com/fabricioferreira I'll be honest, I completely forgot about this. My apologies!

Looking back over it I'm fine with it with the changes. The only minor thing I think would be better would be to change the SubscribeTask methods to simply be called Subscribe; no reason they can't simply be an overload of the method and cleans up the public-facing surface a bit.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/upta/pubsub/pull/20?email_source=notifications&email_token=ADD2WD3VN7AQOMC2BG5UJL3QAHHZFA5CNFSM4G5VTD2KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2L3D3Q#issuecomment-513257966, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ADD2WD7WHMFJCXKOZDMVR6TQAHHZFANCNFSM4G5VTD2A.

fabricioferreira commented 5 years ago

The solution sound good to me. Looking forward for this merge @TomaszStanisz @upta . Thanks!!

upta commented 5 years ago

I'll get it merged and updated on nuget :). Been a bit swamped lately, but I'll try to do so in the next couple of days.

Thanks!


From: Fabricio Carvalho Ferreira notifications@github.com Sent: Monday, August 12, 2019 4:49:24 AM To: upta/pubsub pubsub@noreply.github.com Cc: Brian upta@outlook.com; Mention mention@noreply.github.com Subject: Re: [upta/pubsub] Add SubscribeTask, PublishAsync (#20)

The solution sound good to me. Looking forward for this merge @TomaszStaniszhttps://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FTomaszStanisz&data=02%7C01%7C%7Cb55daf98b5754b9707de08d71f1b1fb3%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637012073660756550&sdata=Vv6jn5mu%2BfkjT8bI9PQw84iEoMcuYuqsA8VeTZDZ4lQ%3D&reserved=0 @uptahttps://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fupta&data=02%7C01%7C%7Cb55daf98b5754b9707de08d71f1b1fb3%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637012073660766561&sdata=nOSWFBAqsfuGPPXHa74vSk1NePL4o3jqxZSvSIzpmJM%3D&reserved=0 . Thanks!!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fupta%2Fpubsub%2Fpull%2F20%3Femail_source%3Dnotifications%26email_token%3DAAZIJDEKD4MI36CVGBDJSEDQEFE4JA5CNFSM4G5VTD2KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4CJDKA%23issuecomment-520393128&data=02%7C01%7C%7Cb55daf98b5754b9707de08d71f1b1fb3%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637012073660776560&sdata=dCRyNctVKG5q3Al0yXD%2FuAb9uKhgOFJxLQPAsqNEREI%3D&reserved=0, or mute the threadhttps://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAAZIJDEEHOMXRWXQ6QGWJXLQEFE4JANCNFSM4G5VTD2A&data=02%7C01%7C%7Cb55daf98b5754b9707de08d71f1b1fb3%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637012073660786565&sdata=8skRjffBZRpy%2FR8%2BqGK87NYtVZS2fTimvtgUQqUofe0%3D&reserved=0.

upta commented 5 years ago

Finally managed to remember long enough and carve out enough time to get this updated ;) It's been merged into master and published on Nuget a 3.1.0.

Thanks!