Fitbit.NET
Fitbit.NET copied to clipboard
aarondcoleman
OAuth2 namespace removal
I'm not a fan of it being in a separate namespace.
Don't see what it gives us ... ?
I don't disagree but the reason why I'm not a fan is the readability of it with the namespace as well makes the naming of it look odd eg Fitbit.Api.Portable.OAuth2.OAuth2Helper. So its the OAuth2 OAuth2 helper? It should either be Fitbit.Api.Portable.OAuth2Helper or the Fitbit.Api.Portable.OAuth2.Helper.
Thoughts?
I agree the repeat wording in the namespace is not ideal, but consumers of the library will most likely not use the fully qualified name when instantiating classes, so they will only see OAuth2Helper. If we remove the OAuth2 prefix, then we will remove clarity as well -- consumers will only see Helper. A helper for what?
In summary, we gain structure in our code base (sorely needed), isolates a subset of the code that should be isolated, and adds clarity of intent (consumers of the library know that all these classes are related to OAuth2) at the cost of an additional namespace.
I believe it is well worth it.
I don't disagree about the code base structure but I believe this is similar to the discussion we had with regards to the FitbitClient.cs size - structure to the source of the library is for us to aid with development is different to namespacing we expose in the consumer of the library. If anything the namespace should be something like Authorization or Security or something more encompassing of everything and then the OAuth1 related classes should be in it as well. Then the name OAuth2Helper makes sense.
@aarondcoleman Tried out the "Security" namespace option route and tbh I'm not 100% sure what it gives us. I still think I prefer the folder structure in the solution to give structure but keep all the classes in the "Fitbit.Api.Portable" namespace. I think with the subtle renaming of some of the classes/interfaces I think it will be clearer - https://github.com/WestDiscGolf/Fitbit.NET/tree/issue-134
Thoughts?