Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API does not have a well-designed OO interface #2

Open
allardhoeve opened this issue May 4, 2017 · 0 comments
Open

API does not have a well-designed OO interface #2

allardhoeve opened this issue May 4, 2017 · 0 comments

Comments

@allardhoeve
Copy link

Hey Xolphin, thanks for opening up a Python API client!

I do have a couple problems with the client code however, let me start with one: the API is not very pythonesque.

Example: when working with the responses the API client returns (Request objects), the attributes the Request object has depends on the reponse of the server. In casu:

if not self.is_error():

        if not self.is_error():
            if 'id' in data: self.id = data['id']
            if 'domainName' in data: self.domain_name = data['domainName']
            if 'subjectAlternativeNames' in data: self.subject_alternative_names = data['subjectAlternativeNames']
            if 'years' in data: self.years = data['years']
            if 'company' in data: self.company = data['company']
            if 'dateOrdered' in data: self.date_ordered = data['dateOrdered']
            if 'department' in data: self.department = data['department']
            if 'address' in data: self.address = data['address']
            if 'zipcode' in data: self.zipcode = data['zipcode']
            if 'city' in data: self.city = data['city']
            if 'province' in data: self.province = data['province']
            if 'country' in data: self.country = data['country']
            if 'reference' in data: self.reference = data['reference']
            if 'approverFirstName' in data: self.approver_first_name = data['approverFirstName']
            if 'approverLastName' in data: self.approver_last_name = data['approverLastName']
            if 'approverEmail' in data: self.approver_email = data['approverEmail']
            if 'approverPhone' in data: self.approver_phone = data['approverPhone']
            if 'requiresAction' in data: self.requiresAction = data['requiresAction']
            if 'brandValidation' in data: self.brandValidation = data['brandValidation']

A class instance of Request normally has loads and loads of attributes, but suddenly has none whatsoever when an error occured.

This makes the class hard to work with. For an example, take your own Base response object:

if hasattr(self, 'errors'):

    def is_error(self):
        if hasattr(self, 'errors'):
            return True
        else:
            return False

The approach is not only very verbose, it is also error-prone, because in the same class:

    def get_error_message(self):
        return self.message

    def get_error_data(self):
        return self.errors

Both of which will fail if the class encountered no message or no errors! These should have been written as:

    def get_error_message(self):
        return getattr(self, "message", None)

    def get_error_data(self):
        return getattr(self, "errors", None)

Please consider simplifying the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant