2
\$\begingroup\$

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)
Sᴀᴍ Onᴇᴌᴀ's user avatar
Sᴀᴍ Onᴇᴌᴀ
29.6k16 gold badges46 silver badges203 bronze badges
askedMay 2, 2023 at 19:19
Ann Nguyen's user avatar
\$\endgroup\$
2
  • 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\$CommentedMay 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\$CommentedJun 4, 2023 at 2:11

1 Answer1

1
\$\begingroup\$
        self.api_version = 'v1'

No need to create lots of object attributes,when a local variable would do.(The variable nameis 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 ausernameof"" 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 ussomething 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 aredistinct methods.Keep that organization.


repeated URLs and URL parents

You have several methods that assignurl 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 theurl temp var,preferring to just hand a catenation expressionin to.get or.post.

Consider defining a manifest constantofDOMAINS = '/dataProduct/domains/'.Or defining a trivial helper for that popular prefix.

This client library sometimes catenates'/dataProduct/domains/' + id.On the server side, aflasklibrary would spell it this way:'/dataProduct/domains/{id}'.Consider adopting such a convention,and parsing out the "variable" portion of each URL.Remember thatinspectis available to you.If a bunch of methods really are identicalexcept for the URL, it should be possibleto 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 severalURLs immediately beneath it.

Your.post /.put methods havecommonkw + data needs, which couldbe 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 iseasily understood and maintained.

Don't sell yourself short on those "verbose" URLstrings that seem to trouble you.One of the first things a maintainerchasing a bug will need to do isgrepfor 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 computessomething useful, such as the root of an equation.And it also wants to tailor its action accordingto who called it.It's not hard to learn thename of the caller.

import inspectdef 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_namedef fn():    root, name = sqrt_and_who_called_me(2.0)    assert name == "fn"

Instead offn we might have

  • def domains():
  • def domains_id():
  • def domains_a():
  • def domains_b():

and a helper wants to know that we're inthe domains hierarchy,or wants to know whether there's a suffixand that it's one of {id, a, b}.

The caller could divulge such naming detailsas a parameter passed into the helper,but that might not be very DRY.(For examplenamedtuplesuffers from the wholeBond = namedtuple('Bond', ...)James Bond syndrome.)Choosing structured names for several GET / POSTroutines could let the helper tailor itsbehavior for each one while minimizing repetition.And of course@decorators are another tool in your toolbox.

Examiningframe.f_back.f_back lets younavigate up the stack to the caller's caller,and so on.

answeredMay 2, 2023 at 20:50
J_H's user avatar
\$\endgroup\$
3
  • \$\begingroup\$Thanks so much for your input!\$\endgroup\$CommentedMay 4, 2023 at 21:02
  • \$\begingroup\$Regarding using inspect, can you give an example? I am not too familiar with using inspect.\$\endgroup\$CommentedMay 4, 2023 at 22:59
  • \$\begingroup\$A call to.currentframe() can provide some useful context.\$\endgroup\$CommentedMay 5, 2023 at 0:25

You mustlog in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.