Refactoring of a client API to avoid duplicated code and unclear passage of parameters

I need to develop an API. The functions of the API are requests that call the services exposed by a server.

Initially the API worked like this:

```
import sys
reload(sys)
sys.setdefaultencoding('utf-8')

import logging
import suds
import json
import string
import socket
import signal
import traceback
from suds.client import Client
from suds.plugin import MessagePlugin
from suds.sudsobject import asdict
from suds.bindings.binding import Binding
from collections import namedtuple

from tornado import gen
from tornado.escape import json_decode, json_encode
from tornado.log import enable_pretty_logging
from tornado.netutil import bind_sockets
from tornado.options import define, options, parse_command_line

import time
import tornado.ioloop
from tornado.concurrent import run_on_executor
from concurrent.futures import ThreadPoolExecutor
import tornado.locale
import tornado.netutil
import tornado.tcpserver
import tornado.web
import tornado.websocket
from tornado.web import authenticated

class Server:
def __init__(self):
self.g_env_el = ""
self.request = ""
self.response = ""

self.id = ""
self.seqno = ""
self.sessionid = ""

self.server_status = 0
self.server_amount = 0
self.server_event = ""
self.server_error = ""
self.server_user = ""
self.server_seqNo = 0

self.client = self.create_client_Server()

def create_client_Server(self):
logging.info("::START - create_client_Server");
url = "http://"+options.server_ip+"/axis2/services/"
wsdlurl = url + 'BrueBoxService?wsdl'
client = Client(url=wsdlurl, location=url+'BrueBoxService', plugins=[SoapFixer(self)])
client.set_options(port='BrueBoxPort')
logging.info("::END - create_client_Server");
return client

@gen.coroutine
def startPayment(self, amount):
"""Ask the server to start the payment process and sets the amount to pay
returns an

For a more generic approach, I wouldn't have introduced the Request class. It just add up on complexity and does not solve the issue of having a callXXX/doXXX style of repetition.

If the sole purpose of the callXXX method is to add logging before and after doXXX execution… then you should just log traces in doXXX which is already the case.

Instead, I would build on your idea of using the bound method to call as a parameter of Server.send, only keep doXXX methods in Asynch and rename them XXX, and use variable number of arguments in Server.send to handle genericity.

Something along the lines of:

class Server:
    @gen.coroutine
    def send(self, send_request, **kwargs):
        """Send an async soap request"""
        logging.info("START - sendRequest")
        try:
            self.g_env_el = 'ChangeRequest'
            kwargs['server'] = self
            kwargs['client'] = self.client
            response =  yield send_request(**kwargs)
        except Exception as e:
            logging.error("exception %s" %e)
            response = 0
        logging.info("END - sendRequest")
        raise gen.Return(response)

This will require that every method of Asynch defines a parameter named client and an other one named server.

You then use it with:

server = Server()
asynch = Asynch()
server.send(asynch.startPayment, amount=3)
server.send(asynch.stub_method, stub_param_1='a', stub_param_2=10)

And so on.

Now, for the rest of your code:

• Docstrings appears after the method declaration, not before.

• Remove the spaces around the = sign for parameters with default values.

• Use snake_case for both variables and functions names.

• Do not remove if __name__ == "__main__", it is good practice.

• Use format string syntax whenever possible:

• url = "http://{}/axis2/services/BrueBoxService".format(options.server_ip)

• Not sure if logging supports it, but at least it supports logging.error("exception %s", e) (I guess that your new _exceptionOccurred, which you’re not showing, still uses it).

• Remove pass statements in non-empty methods.

StackExchange Code Review Q#114623, answer score: 2