HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Python wrapper for the Help Scout API

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thehelpwrapperforpythonapiscout

Problem

I started porting an API wrapper from Java to Python for practice. I am looking for ways to improve the readability/maintainability this code.

I have done some reading about "pythonic" style and I am hoping someone could provide some feedback as to how one goes about making their code more pythonic.

All source can be found here.

The models module is what contains the custom types that the parser creates.

```
import requests
import json
import base64
import models

class ApiClient:
BASE_URL = "https://api.helpscout.net/v1/"
apiKey = ""

def getMailbox(self, mailbox_id, fields=None):
url = "mailboxes/" + str(mailbox_id) + ".json"
if fields != None:
url = self.setFields(url, fields)
return self.getItem(url, "Mailbox", 200)

def getMailboxes(self, fields=None):
url = "mailboxes.json"
if fields != None:
url = self.setFields(url, fields)
return self.getPage(url,"Mailbox", fields)

def getFolders(self, mailbox_id, fields=None):
url = "mailboxes/" + str(mailbox_id) + "/folders.json"
if fields != None:
url = self.setFields(url, fields)
return self.getPage(url, "Folder", 200)

def getConverstationsForFolders(self, mailbox_id, folder_id, fields=None):
url = "mailboxes/" + str(mailbox_id) + "/folder/" + str(folder_id) + "conversations.json"
if fields != None:
url = self.setFields(url, fields)
return self.getPage(url, "Conversation", 200)

def getConversationsForMailbox(self, mailbox_id, fields=None):
url = "mailbox/" + str(mailbox_id) + "/conversations.json"
if fields != None :
url = self.setFields(url, fields)
return self.getPage(url)

def getConversationForCustomerByMailbox(self, mailbox_id, customer_id, fields=None):
url = "mailboxes/" + str(mailbox_id) + "/customers/" + str(customer_id) + "/conversations.json"
if fields != None:

Solution

Depends on your definition of "pythonic", if you haven't already check out already PEP 20, "The Zen of Python" gives some good pragmatic guidelines and PEP 8 gives some style guidelines.

Personally, pythonic or not, I like things to be easy to read and concise, so here are a couple suggestions

def getFolders(self, mailbox_id, fields=None):
    # if there's no mailbox_id shouldn't this throw an Exception?
    url = "mailboxes/%s/folders.json" % mailbox_id
    if fields:
        url = self.setFields(url, fields)
    return self.getPage(url, "Folder", 200)


Since this doesn't depend on anything in the class and looks like a utility method, you could just move it outside of the class completely and omit self and you might want to check if the url is None

def setFields(url, fields):
    # checks None and length
    if not fields:
        return url
    # if speed is important, you can concatenate instead
    # return url + "?fields=" + ",".join(fields);
    return "%s?fields=%s" % (url, ",".join(fields))


Here's an alternative to the if/else statements to simulate case/switch:

def checkStatus(self, code, expected):
    if code == expected:
        return
    def error_codes(code):
        messages = { 
            400 : "The request was not formatted correctly",
            401 : "Invalid API Key" }
        default_message = "API Key Suspended"
        return messages.get(code, default_message)
    raise Exception(error_codes(code))

Code Snippets

def getFolders(self, mailbox_id, fields=None):
    # if there's no mailbox_id shouldn't this throw an Exception?
    url = "mailboxes/%s/folders.json" % mailbox_id
    if fields:
        url = self.setFields(url, fields)
    return self.getPage(url, "Folder", 200)
def setFields(url, fields):
    # checks None and length
    if not fields:
        return url
    # if speed is important, you can concatenate instead
    # return url + "?fields=" + ",".join(fields);
    return "%s?fields=%s" % (url, ",".join(fields))
def checkStatus(self, code, expected):
    if code == expected:
        return
    def error_codes(code):
        messages = { 
            400 : "The request was not formatted correctly",
            401 : "Invalid API Key" }
        default_message = "API Key Suspended"
        return messages.get(code, default_message)
    raise Exception(error_codes(code))

Context

StackExchange Code Review Q#16282, answer score: 3

Revisions (0)

No revisions yet.