7
\$\begingroup\$

This code is a class I use in my financial market trading bot (note: I'm a daytrader-come-beginner programmer, not the other way around).

Although I have the application running successfully, it has major design flaws which I intend to now address ASAP.

An API requires that a unique transaction ref is submitted with each new order, otherwise, that order will be rejected. I have very little idea if this is a competent or robust design, I could not find any guidance on the issue elsewhere.

I'm very much open to refactoring or a completely new design.

class UniqueTransactionRef:
    """Transaction reference number iterator 

    Loads, generates and saves unique transaction ref data for ``Bot`` orders

    :ivar file: absolute path the the .json transaction ref data file
    :ivar num: an integer representing the last generated transaction ref number
    """

    default_file = r"C:\Bot\Bot\trans_ref.json"

    def __init__(self, file=None):
        """Initialize the UniqueTransactionRef object

        :param file: an absolute path to a .json file containing the transaction
        reference number, as an integer
        """
        self.file = file if file else self.default_file
        self.num = 0
        self.load()

    def __iter__(self):
        return self

    def __next__(self):
        """Increment the self.num attribute. Save the value to the local file.

        :returns: a string formatted version of the self.num class attribute
        """
        self.num += 1
        self.save()
        return str(self.num)

    def get(self):
        """Call the ``next`` function on self """
        return next(self)

    def load(self):
        """Load the transaction ref data from the local file

        :raises errors.UniqueTransactionRefError: Cannot find file
        """
        try:
            with open(self.file) as f:
                self.num = json.load(f)
        except FileNotFoundError:
            raise errors.UniqueTransactionRefError(
                self.file,
                num=self.num,
                message="Cannot find file",
                )

    def save(self):
        """Save self.num to the designated local file

        :raises UniqueTransactionRefError: 
        """
        try:
            with open(self.file, "w") as f:
                json.dump(self.num, f)
        except Exception as e:
            raise errors.UniqueTransactionRefError(self.file,
                                                     num=self.num,
                                                     message=e)
unique_trans_ref = UniqueTransactionRef()  # module level variable

Simplified example usage:

class Bot:
    def __init__(self, unique_trans_ref):
        self.unique_trans_ref = unique_trans_ref

    def construct_order():
        trans_ref = next(self.unique_trans_ref)
\$\endgroup\$
5
\$\begingroup\$

The server requires a unique ID to detect replays in the case of network failures, support idempotency, and to report order confirmations in a way the client can make sense of. So yes, the server offers a sensible API.

You can send sequential numbers, or a random guid. Consider using import uuid.

Consider using a numeric prefix of yyyymmdd, with strftime('%Y%m%d') + '%06d' % num. This will help when you have issues with deleted file, backups, switching client laptops, and so on. It makes the transaction IDs more human readable. (It also makes them easier to guess, if you're concerned about a possible attacker.)

Thank you for using with for I/O. Consider acquiring a lock before saving updated counter, if you're concerned that multiple daemons or users might run instances of your code concurrently.

There's no need for default_file, as you can just put that default in the signature:

    def __init__(self, file=r"C:\Bot\Bot\trans_ref.json"):

You catch a subset of the things that can go wrong with file I/O and JSON parsing. It does not appear well motivated, given that the client doesn't try anyway, so it doesn't care whether it gets fatal exception A or B. Perhaps you have a use case which requires a custom exception, but you have not shown it. Likely you don't need it at this point. I would just let library load and open throw as they will, and caller will (correctly) receive fatal exception if one happens. Later, when caller implements an actual error recovery strategy, you will have new requirements in hand for what exception details must be returned, and you can make a better informed design decision about how UniqueTransactionRef should interact with exceptions.

Write the simplest code you can get away with (it's easier to test!). When you get a new concrete requirement in hand, make the code a little fancier.

\$\endgroup\$
  • \$\begingroup\$ Just for the record; str(uuid4()) is 400x faster than my implementation. My application creates approx 3000 orders per day, that's 13 secs of processing time saved. \$\endgroup\$ – Dave Feb 27 '18 at 7:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.