patternpythonMinor
Python wrapper for the Help Scout API
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:
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
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
Here's an alternative to the if/else statements to simulate
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 Nonedef 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.