4
\$\begingroup\$

The callable class below loads configuration data, formats it into a form compatible with my application(from its human-friendly version), performs some important checking, and finally makes it available to be accessed by calling the instance.

This is part of my(working) financial market trading application, written after 10 months of part-time programming. I'm trying to address it's many, many design flaws. One-by-one.

The BotOrderData class is part of my attempt to move all configuration data out of the source code into a single configuration location. There is, therefore, a requirement for several of these types of classes(or whatever ends up being the best solution). For example, I'm often required to change parameters such as:

  • pricing
  • market volatility tolerances
  • the times at which the application should refrain from trading

I'd like to know if this is a competent design choice, of any alternatives, and of any other smaller refactorings.

Code:

class BotOrderData():
 """A class for loading, formatting, checking, storing ``Bot`` pricing data
 The BotOrderData class wraps a dict which holds the various pricing
 parameters after loading, formatting and checking functions are performed.
 This data is accessed by instance of ``Bot``.
 :ivar DEFAULT_PARAMS: a dict of human-friendly pricing configuration data
 to be used in the absence of local configuration file data.
 """
 #
 # TO DO:
 # Finish docstrings
 #
 # The default bot pricing data, should none be available from a local
 # config file listed in self.config_locations
 DEFAULT_PARAMS = {
 "S": {"EUR_USD": {"trigger": 1.2, "upper": 2.0, "ignore": 4.999},
 "GBP_USD": {"trigger": 1.2, "upper": 2.0, "ignore": 4.999},
 "EUR_GBP": {"trigger": 1.2, "upper": 2.0, "ignore": 4.999},
 },
 "B": {"EUR_USD": {"trigger": 0.0, "upper": -2.0, "ignore": 4.999},
 "GBP_USD": {"trigger": 0.0, "upper": -2.0, "ignore": 4.999},
 "EUR_GBP": {"trigger": 0.0, "upper": -2.0, "ignore": 4.999},
 }
 }
 def __init__(self):
 """Initialize the Triggers object
 :ivar params: a dict which holds the bot order parameters
 :ivar config_name: a str. The basename of the config file to search the
 system for.
 :ivar config_locations: an iterable of folders locations which may
 contain a qualifying config file. To be listed in the order in which
 they should take precendence. Eg; that listed first takes priority
 """
 self.params = dict()
 self.config_name = "bot_config_data.py"
 # List locations in order of highest priority first. Eg; if the
 # first location exists, it's data will be loaded and used for
 # configuration
 self.config_locations = (
 r"C:\Users\Dave\Bot",
 os.path.expanduser("~trade_bot/"),
 )
 self.add_param_data()
 def __call__(self, action: str, mkt_id: str):
 """Query and return pricing parameter data
 :returns: A tuple of three float values in order
 ("trigger", "upper", "ignore")
 """
 return (self.params[action][mkt_id]["trigger"],
 self.params[action][mkt_id]["upper"],
 self.params[action][mkt_id]["ignore"],
 )
 def discover_locations(self):
 """Return a list of configuration file locations"""
 logging.debug("self.config_locations: {}".format(self.config_locations))
 locations = [os.path.join(path, self.config_name)
 for path in self.config_locations]
 exist = [l for l in locations if os.path.exists(l)] 
 return exist
 def get_config_from_file(self):
 """Load data from the first configuration file in available locations
 """
 data = {}
 locations = self.discover_locations()
 if not locations:
 return None
 with open(locations[0]) as f:
 code = compile(f.read(), locations[0], "exec")
 exec(code, globals(), data)
 return data["price_data"]
 def process_params(self, params: dict):
 """Convert the human-friendly config data -> ``Bot`` friendly version
 :param: params: a dict of config data, either from a local config file
 or from DEFAULT_PARAMS if the former is not present
 """
 sell_mkt = params["S"]
 buy_mkt = params["B"]
 for s_mkt in sell_mkt:
 sell_mkt[s_mkt]["trigger"] = 1.0 + sell_mkt[s_mkt]["trigger"]/100
 sell_mkt[s_mkt]["upper"] = 1.0 + sell_mkt[s_mkt]["upper"]/100
 for b_mkt in buy_mkt:
 buy_mkt[b_mkt]["trigger"] = 1.0 + buy_mkt[b_mkt]["trigger"]/100
 buy_mkt[b_mkt]["upper"] = 1.0 + buy_mkt[b_mkt]["upper"]/100
 return params
 def add_param_data(self):
 """Manager method which adds pricing parameters to self.params"""
 file_config = self.get_config_from_file()
 params = self.DEFAULT_PARAMS if not file_config else file_config
 params = self.process_params(params)
 self.check_params(params)
 self.params.update(params)
 def check_params(self, params: dict):
 """Check the params data is valid. This check must ALWAYS pass.
 :param params: a nested dict of Bot pricing parameters 
 """
 checks = list(
 [e for e in params["B"] if params["B"][e]["trigger"] > 1.00
 or params["B"][e]["upper"] > params["B"][e]["trigger"]
 or params["B"][e]["ignore"] < 0]
 +
 [e for e in params["S"] if params["S"][e]["trigger"] < 1.00
 or params["S"][e]["upper"] < params["S"][e]["trigger"]
 or params["S"][e]["ignore"] < 0]
 )
 assert not checks, (self.__class__.__name__
 + " contains invalid data: {}".format(params))
bot_pricing_data = BotOrderData() # module level variable

Example usage:

from bot_prices import bot_pricing_data
class Bot():
 def __init__():
 self.trigger, self.upper, self.ignore =\
 bot_pricing_data(self.action , self.mkt_id) # eg ("B", "EUR_USD")
200_success
146k22 gold badges191 silver badges481 bronze badges
asked Feb 28, 2018 at 13:40
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

will drop you a couple considerations

You may want to store the default parameters on a separate settings file, or even better, on a YAML file, which will ease the tasks to read and update this configuration by you or someone without much knowledge in Python.

Some more info about how to integrate configuration in YAML into a python project: https://pyyaml.org/wiki/PyYAMLDocumentation

self.config_locations you definitely want to extract onto a settings file, since in the future it may change when you execute the program on a different machine, then you just load this settings from wherever with different paths.

add_param_data may be renamed to load_configuration, or load_settings, which is more explicit to what happens. And ideally the logic to discern from where the parameters are coming from is done outside, and you call this method with a parameter, which will be the path to where default parameters are (file or dict or YAML..)

check_params and process_params have repeated code, you can simply have a function where you execute the same code depending on a parameter as well

instead of:

 sell_mkt = params["S"]
 buy_mkt = params["B"]
 for s_mkt in sell_mkt:
 sell_mkt[s_mkt]["trigger"] = 1.0 + sell_mkt[s_mkt]["trigger"]/100
 sell_mkt[s_mkt]["upper"] = 1.0 + sell_mkt[s_mkt]["upper"]/100
 for b_mkt in buy_mkt:
 buy_mkt[b_mkt]["trigger"] = 1.0 + buy_mkt[b_mkt]["trigger"]/100
 buy_mkt[b_mkt]["upper"] = 1.0 + buy_mkt[b_mkt]["upper"]/100

You could have

sell_market = params["S"]
_update_market(sell_market)

Then in another method

def _update_market(market):
 for m in market:
 market[m]["trigger"] = 1.0 + market[m]["trigger"]/100
 market[m]["upper"] = 1.0 + market[m]["upper"]/100

Also, in my opinion, discover_locations should be renamed to get_location_list

Just to end, this is how you can design the __init__ method. Just an idea

def __init__():
 file = get_configuration_file()
 params = load_parameters(file)
 validate_parameters(params) # This will be check_params, that in the end can update self.params

Hope you get some valid comments from all of this

Enjoy coding!

answered Feb 28, 2018 at 22:34
\$\endgroup\$
1
  • \$\begingroup\$ What do you think about having those three statements in __init__() placed in a manager method for ease of testing? A single method can then Mocked as opposed to three? I have little confidence in my testing methods though. \$\endgroup\$ Commented Mar 2, 2018 at 7:03

You must log 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.