Refactor and optimize code
@warrior-abhijit has suggested several changes to vcert-python code. We can address them together in one issue.
switch case will be better here ? Originally posted by @warrior-abhijit in https://github.com/Venafi/vcert-python/pull/41#discussion_r488175192
address todo now ?? as these are lot of if, else in here Originally posted by @warrior-abhijit in https://github.com/Venafi/vcert-python/pull/41#discussion_r488176457
regex match API would be lot better here and will remove lot of duplicate code below w.r.t regex match Originally posted by @warrior-abhijit in https://github.com/Venafi/vcert-python/pull/41#discussion_r488177162
switch case may be here as well ? Originally posted by @warrior-abhijit in https://github.com/Venafi/vcert-python/pull/41#discussion_r488177862
There is a handy Python wrapper called @property. This can be handy here. It would look like this: @property def base_url(self): # This is a getter return self._base_url
@base_url.setter def base_url(self, value): # This is the setter method self._base_url = self._normalize_and_verify_base_url(value)
It's nicer for refactoring and is pretty explicit. Originally posted by @HELGAHR in https://github.com/Venafi/vcert-python/pull/41#discussion_r492452816
How safe is it in this method to assume that these dictionary keys resolve? I'm new to this code, but I usually think thrice before trying to access a node in the dictionary without .get(). Originally posted by @HELGAHR in https://github.com/Venafi/vcert-python/pull/41#discussion_r493173154
Just a tidbit of input: Python string objects have a .startswith() method that's easier to read than a regex, although a regex works fine. Originally posted by @HELGAHR in https://github.com/Venafi/vcert-python/pull/41#discussion_r493173571
No use in having a doc string if the parameters aren't described, IMO. Originally posted by @HELGAHR in https://github.com/Venafi/vcert-python/pull/41#discussion_r493174068