search

schmorrison / Zoho

Golang API for Zoho services
MIT License
35 stars 34 forks source link

Discuss: Implement zohoTLD field and SetZohoTLD method #15

Closed schmorrison closed 4 years ago

schmorrison commented 5 years ago

Would it be more appropriate to move ZohoTLD and the SetZohoTLD method into the API struct from zoho/crm/crm.go? This would allow us to make the ZohoTLD field private as zohoTLD.

Because the Zoho struct is embedded into the API struct this would prevent accessing the field inappropriately, but would require adding this field and method to the API struct of every subpackage.

Again, not sure its worth it, but I will create a new issue where we can discuss it.

Originally posted by @schmorrison in https://github.com/schmorrison/Zoho/pull/14#issuecomment-506008172

schmorrison commented 5 years ago

Seems to me the overhead of implementing the ZohoTLD and SetZohoTLD method in each subpackage is more effort than its worth. In situations where a use does not need to change TLD, or the Zoho services does not provide a reason to do so, the user will just continue using the default TLD.

However when a user changes the TLD via the current method, and doesn't require a changed TLD when using a different Zoho subpackage ( Books doesn't seem to have a separate set of TLDs for each region) will be regularly switching back and forth. I think changing the ZohoTLD and SetZohoTLD method into the API struct of services that require these changes would be easier to manage in edge cases.

@meyskens -- Note these changes to be made so your workflow doesn't suffer. Thanks

schmorrison

PS. I stepped back for a second because I cannot evaluate the overreach of this function and variable properly. Part of me wants to change it, but I also recognize that it is esentially proper already.