terrasnek icon indicating copy to clipboard operation
terrasnek copied to clipboard

handle query strings via requests() parameters

Open evilensky opened this issue 2 years ago • 0 comments

In _get() the query string is constructed via string concatenation. Can we replace with requests params dicts instead? I believe this is proper prima facie but below I've shared my reasons for future benefits as well.

To replace the custom string concatenation for params with regular requests parameters would bring benefits:

  • As an immediate benefit, the ordering of the string concatenation would no longer be significant, requests will process dict-like objects into parameters correctly with no additional processing needed, eliminating string processing and concatenation logic and reducing surface area for needed unit tests.
  • Converting to use dict-like objects as requests params will give us the ability to in the future create possibly type hinted objects or classes which will be processed to_dict() such as include, page, sort, and relations, and perform client-side validation of API parameters before calling _get(). The benefits of bringing client-side validation to parameters are discussed in #34
  • API-specific _get() options like found here can move to their own objects to take advantage of the mentioned type hinting / input parameter validation, in the future.

An example of how this would work can be found in this commit.

evilensky avatar Nov 25 '22 16:11 evilensky