Python Making Methods Modular and DRY

Question

I'm learning object oriented programming and am still trying to understand the best way to structure a class for modularity and attempting to follow DRY principles. I currently created a class containing multiple requests that share the same self.reponse block repeatedly. I am not sure the best way to abstract this outside of each function call since I am also returning the response object within each method.

My code currently looks like this:

import requests
import urllib3
import logging
import re
from itertools import zip_longest

log = logging.getLogger()

server = "https://example.com"
organization = "/example/"
username = "[email protected]"
PAT = "xxxxxxxxxxxxxxxxxxxxx"

urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning)


class InvokeRequest():
    def __init__(self):
        self.server = server + organization

    # Get All Repositories
    def getRepositories(self, project):
        self.URL = self.server + project + "/_apis/git/repositories"
        self.response = requests.get(self.URL, auth=(username, PAT), verify=False)
        return self.response.json()

    # Get All Build Definitions
    def getBuildDefinitions(self, project):
        self.URL = self.server + project + "/_apis/build/definitions/"
        self.response = requests.get(self.URL, auth=(username, PAT), verify=False)
        return self.response.json()

    # Get Specific Build Definition
    def getBuildDefinitionID(self, project, ID):
        self.URL = self.server + project + "/_apis/build/definitions/" + str(ID)
        self.response = requests.get(self.URL, auth=(username, PAT), verify=False)
        return self.response.json()

    # Get All Release Definitions
    def getReleaseDefinitions(self, releaseID):
        self.URL = self.server + releaseID + "/_apis/git/releaseID"
        self.response = requests.get(self.URL, auth=(username, PAT), verify=False)
        return self.response.json()


request = InvokeRequest()

repos = request.getRepositories("DevOps")

list_of_repos = []
for repo in repos["value"] or []:
    list_of_repos.append(repo["name"])

buildDefinitions = request.getBuildDefinitions("DevOps")

build_definition_IDs = []
build_pipeline_names = []
for build in buildDefinitions["value"]:
    if build["queueStatus"] == "enabled":  # Skip over disabled builds
        build_definition_IDs.append(build["id"])
        build_pipeline_names.append(build["name"])

associated_build_repos = []  # Repos containing an associated build pipeline
for ID in build_definition_IDs:
    buildDefinition = request.getBuildDefinitionID("DevOps", ID)
    log.debug(buildDefinition["repository"].get("name"), "has pipeline")
    try:
        associated_build_repos.append(buildDefinition["repository"].get("name"))
    except KeyError as e:
        log.error(e)

For now, I am only passing a single project "DevOps", however, this will change in the future. I want to find a way to prevent repeating this specific block of code:

self.response = requests.get(self.URL, auth=(username, PAT), verify=False)
return self.response.json()

I have considered creating a base class called InvokeRequestConfig() or something similar to create the setup of the server and response object, that way I could inherit that class in InvokeRequest(). however I think that adds a lot of complexity that might not be necessary.

What is the best way to reuse the code above to make it more modular and follow best practices of OOP and DRY?

Answer 1

All the methods in InvokeRequest are basically the same. The only part that differs is the URL. Just pass that in instead of passing in the parts of the URL and constructing the URL in the function.

Before I show the function that I came up with though, I'll note that there's no reason InvokeRequest should be a class, and there's also no reason that self.URL and self.response in those methods should be instance attributes. I'm far from practiced in OOP, but making arbitrary bits of code into classes isn't helpful. If you were passing the self.server data into the constructor, then maybe there would be some justification. As it stands now though, I'm going to get rid of the class and instance attributes.

To start out with, here's the basic part that was repeated in all the methods:

def get_response(url):
    response = requests.get(url, auth=(username, PAT), verify=False)
    return response.json()

It now expects that the url will be passed in like I mentioned. You can ease making the URL though using a helper:

def build_url(*parts):
    return BASE_URL + "".join(parts)

If you haven't encountered var-args before, *parts just means that the function accepts as many arguments as you want, and bundles them into a tuple.

Now you can write:

json = get_response(build_url(project, "/_apis/git/repositories"))  # Was getRepositories
json = get_response(build_url(releaseID, "/_apis/git/releaseID"))  # Was getReleaseDefinitions
json = get_response(build_url(project, "/_apis/build/definitions/", str(ID)))  # Was getBuildDefinitionID

You could also put the call to build_url inside get_response and have *parts as the parameter to get_response:

def get_response(*parts):
    url = build_url(*parts)
    response = requests.get(url, auth=(username, PAT), verify=False)
    return response.json()

json = get_response(project, "/_apis/git/repositories")
json = get_response(releaseID, "/_apis/git/releaseID")
json = get_response(project, "/_apis/build/definitions/", str(ID))

Which you want to use depends on if get_response will always use a URL returned by build_url, or if it can differ.

You could wrap the calls to get_response in some functions too to avoid needing to manually put the URL together each time:

def get_build_definition_ID():
    return get_response(project, "/_apis/build/definitions/", str(ID))

---

Also note the naming convention I used. Python uses snake_case, not camelCase.