Sendgrid-csharp: Library does not follow naming convention for Async methods

[roryprimrose]

Task based methods should use an Async suffix in their name. The current naming is misleading an implies that the code is invoked synchronously.

This would be a breaking change without overloading methods and using the ObsoleteAttribute.

[Jericho]

I agree with you: I also believe it's important to use 'Async' suffix to clearly indicate the asynchronous nature of methods.

I have forked SendGrid's repo and am working on several improvements such as ability to inject httpclient for unit testing, meaningful method names (as opposed to simply 'get', 'post', 'patch', etc.), strongly typed return values from methods, etc. In my fork I am following a naming convention where all methods have the 'Async' suffix.

You can find my fork here: https://github.com/Jericho/sendgrid-csharp

[roryprimrose]

Sweet. Can you also please add CancellationToken parameters into those signatures.

[Jericho]

Excellent idea! I'll add CancellationToken to methods.

[Jericho]

done

[thinkingserious]

@roryprimrose thanks for the feedback! I'm leaving this one open for consideration.

@Jericho thanks for your support!

[thinkingserious]

For reference: https://msdn.microsoft.com/en-us/library/hh873175.aspx

[thinkingserious]

@roryprimrose, @Jericho,

I believe I've made a proper fix, if you have a moment, please take a look:

1. This is the client code that was modified
2. Here is the new working example.

I'm planning to merge this one in today and then update the dependency here. It looks like this will be a breaking change, so I'll likely update the function naming as well.

[edorphy]

I might be missing something, but why is there emphasis in using dynamic objects? We lose all type safety. The v2 API that Azure still publishes an example using, was very type safe and hard to mess up. With the dynamic client, its very easy to mess up something with just a simple typo (plus there is no VS auto complete).

Azure's .Net SDK has some very clean abstractions around formatting push notification json payloads with a very strong API.

[thinkingserious]

@edorphy,

Please take a look at the conversation here: https://github.com/sendgrid/sendgrid-csharp/issues/261 (tl;dr; we will solve the issue you describe within our helpers)

We would love for you to join in on the conversation and help us move the library forward.

[ohadschn]

While we're on the subject of naming convention, C# uses PascalCase for property names so stuff like sg.client.mail.send.post should be sg.Client.Mail.Send.Post(). And stuff like sg.client.api_keys should be sg.Client.ApiKeys. Thanks!

[thinkingserious]

@ohadschn, correct, we have a ticket to track that fix here: https://github.com/sendgrid/sendgrid-csharp/issues/239

I've add a +1 for you internally. Thanks!

[SimonCropp]

we went through a similar process recently and decided to not have an Async suffix http://docs.particular.net/nservicebus/upgrades/5to6-async-suffix

your circumstance may be different. but thought u might find our logic useful

[ohadschn]

@SimonCropp all valid reasons but IMHO being consistent with the BCL is more important

[thinkingserious]

This is resolved in this #303