I am writing a python REST API library for Starburst and am trying to figure the best way to structure the code so it's dry but also easy to understand. Below is an example of code that is not dry.
Other methods I've researched is having a config file with the different API calls and having a single function for get/put/delete/post.
Other way is grouping based on domains and passing the method as a variable.
class StarburstAPI(object):
def __init__(self, username=None, password=None, oauth=None, session=None):
self.api_version = 'v1'
self.base_url = 'https://starburst.com/api/' + self.api_version
self.headers = default_headers()
self.params = {}
if session is None:
session = requests.Session()
self._session = session
if username is not None or password is not None:
self._session.auth = (username, password)
self._session.cookies = self._session.head(self.base_url).cookies
self._session.headers.update({'Content-Type': 'application/json'})
# Get list of domains
def list_domains(self):
url = self.base_url + '/dataProduct/domains/'
return self._session.get(url)
# Get domain by id
def get_domain(self, id: str):
url = self.base_url + '/dataProduct/domains/' + id
return self._session.get(url)
# Create domain
def create_domain(self, data=None, **kw):
url = self.base_url + '/dataProduct/domains/'
if data:
kw = add_json_headers(kw)
data = json.dumps(data)
return self._session.post(url, data, **kw)
# Delete domain by id
def delete_domain(self, id: str):
url = self.base_url + '/dataProduct/domains/' + id
return self._session.delete(url)
# Update domain by id
def update_domain(self, id: str, data=None, **kw):
url = self.base_url + '/dataProduct/domains/' + id
if data:
kw = add_json_headers(kw)
data = json.dumps(data)
return self._session.put(url, data, **kw)
-
2\$\begingroup\$ "trying to figure the best way to structure the code so it's DRY but also easy to understand" -- if you are considering several competing approaches to organizing this code, there's still time to edit the Question, mentioning their pros and cons. Once a reviewer offers an Answer then edits will be frozen. \$\endgroup\$J_H– J_H2023年05月02日 19:42:44 +00:00Commented May 2, 2023 at 19:42
-
\$\begingroup\$ Is this Python 2 or 3? You use Python 2 syntax in your class declaration and the rest is ambiguous. \$\endgroup\$Reinderien– Reinderien2023年06月04日 02:11:47 +00:00Commented Jun 4, 2023 at 2:11
1 Answer 1
self.api_version = 'v1'
No need to create lots of object attributes, when a local variable would do. (The variable name is helpful, thank you.)
api_version = 'v1'
Here's another nit.
if session is None:
session = requests.Session()
self._session = session
This is clear enough. We could express it more compactly with this common idiom:
self._session = session or requests.Session()
Similarly you could write this slightly more compact expression:
if username or password:
self._session.auth = (username, password)
It's not strictly identical, but a username
of ""
empty string likely is of little interest.
You could delete this comment, for example:
# Get list of domains
def list_domains(self):
Avoid # comments
that don't tell us
something beyond what the code already said.
We put the "how", the technical details, into the code. We put the "why" in the comments or method docstring.
http verb
I really like how {list,create}_domain
are
distinct methods.
Keep that organization.
repeated URLs and URL parents
You have several methods that assign url
as this plus that.
It seems to trouble you.
I wouldn't worry about it too much.
As written the code is very clear,
and it's easy for a maintainer to search for matching URLs.
You might remove the url
temp var,
preferring to just hand a catenation expression
in to .get
or .post
.
Consider defining a manifest constant
of DOMAINS = '/dataProduct/domains/'
.
Or defining a trivial helper for that popular prefix.
This client library sometimes catenates
'/dataProduct/domains/' + id
.
On the server side, a
flask
library would spell it this way:
'/dataProduct/domains/{id}'
.
Consider adopting such a convention,
and parsing out the "variable" portion of each URL.
Remember that
inspect
is available to you.
If a bunch of methods really are identical
except for the URL, it should be possible
to define them as one-liners, given appropriate support routines.
A helper for a common parent, such as DOMAINS, may assist in concisely defining accessors for several URLs immediately beneath it.
Your .post
/ .put
methods have
common kw + data
needs, which could
be extracted to a common helper.
Overall? Sure, this code is repetitive. But it's not that bad.
I am reminded of unit test code, where copy-n-paste (the enemy of DRY!) is explicitly accepted as a good practice. Why? To produce simple code that is easily understood and maintained.
Don't sell yourself short on those "verbose" URL
strings that seem to trouble you.
One of the first things a maintainer
chasing a bug will need to do is grep
for a URL or a fragment of one.
The OP expresses URLs in very simple form,
and that makes the codebase easily grep'able.
This code achieves many of its design goals.
I would be willing to delegate or accept maintenance tasks on it.
EDIT
Suppose you have a function that computes something useful, such as the root of an equation. And it also wants to tailor its action according to who called it. It's not hard to learn the name of the caller.
import inspect
def sqrt_and_who_called_me(n):
"""Returns a root, and the name of the calling function."""
frame = inspect.currentframe()
return math.sqrt(n), frame.f_back.f_code.co_name
def fn():
root, name = sqrt_and_who_called_me(2.0)
assert name == "fn"
Instead of fn
we might have
def domains():
def domains_id():
def domains_a():
def domains_b():
and a helper wants to know that we're in the domains hierarchy, or wants to know whether there's a suffix and that it's one of {id, a, b}.
The caller could divulge such naming details
as a parameter passed into the helper,
but that might not be very DRY.
(For example
namedtuple
suffers from the whole
Bond = namedtuple('Bond', ...)
James Bond syndrome.)
Choosing structured names for several GET / POST
routines could let the helper tailor its
behavior for each one while minimizing repetition.
And of course @decorators
are another tool in your toolbox.
Examining frame.f_back.f_back
lets you
navigate up the stack to the caller's caller,
and so on.
-
\$\begingroup\$ Thanks so much for your input! \$\endgroup\$Ann Nguyen– Ann Nguyen2023年05月04日 21:02:43 +00:00Commented May 4, 2023 at 21:02
-
\$\begingroup\$ Regarding using inspect, can you give an example? I am not too familiar with using inspect. \$\endgroup\$Ann Nguyen– Ann Nguyen2023年05月04日 22:59:09 +00:00Commented May 4, 2023 at 22:59
-
\$\begingroup\$ A call to
.currentframe()
can provide some useful context. \$\endgroup\$J_H– J_H2023年05月05日 00:25:29 +00:00Commented May 5, 2023 at 0:25