HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Tracking orders efficiently using a Python class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
trackingefficientlyusingpythonclassorders

Problem

I've written a Ledger class for a daemon process, which is supposed to keep track of an exchange's orderbook. My main concern is the way I keep track of the orders - more precisely, the containers I use to store them: a combination of set() and dict() appeared like the most feasible solution.
However, I'm wondering If haven't missed a sweet feature or nifty recipe that works better.

But first, my code - the lines in question are within the constructor:

```
import logging
from heapq import nlargest, nsmallest

log = logging.getLogger(__name__)

class OrderRemovalError(Exception):
pass

class Ledger:
def __init__(self):
self._ask_orders = {} # Keeps orders by price {price: (amount, count)}
self._ask_ladder = set() # keeps ask prices
self._bid_orders = {} # Keeps orders by price {price: (amount, count)}
self._bid_ladder = set() # keeps bid prices

def _add(self, order, ledger_orders, ledger_oders_ladder):
price, count, amount = order
price_key = str(price)
ledger_orders[price_key] = [amount, count]
ledger_oders_ladder.add(price)
return True

def _remove(self, order, ledger_orders, ledger_order_ladder):
price, amount, count = order
price_key = str(price)
err = []
c = 0
try:
ledger_orders.pop(price_key)
except KeyError:
c += 1
except Exception as e:
err.append(e)

try:
ledger_order_ladder.remove(price)
except ValueError:
c += 1
except Exception as e:
err.append(e)

if c == 2:
pass # order didn't exist.
return False
elif c == 1 or len(err) > 1:
raise OrderRemovalError("Something went wrong during deletion "
"of order %s! Error: %s" % ((*order,), err))
elif c == 0:
log.debug("Order deleted successfully! %s" % (order,

Solution

Some people like to keep their code DRY rather than WET, I would recomend that you KISS.

If we look at The Zen of Python, you'll see:


Simple is better than complex.

What I'm trying to say is your code is hard to read, and you should definitely not start optimising from this base.
As building on a base of sand is always going to lead to structural imperfections.
And then one day it'll fail, and you'll need to start afresh.

So lets first look at how you can improve your base.

  • bids and asks change three things, and so you could merge these.



  • bid and ask have the same interface. Make a class for it.



  • self._bid_ladder is the equivalent of self._bin_orders.keys().



  • Both _add and _remove can be changed to be a couple of lines long by removing the 'ladders'.



  • bid and ask would make more sense if you allowed the new class to have the __getitem__ special method.



And so honestly I'd entirely re-write this.
Without __getitem__ you should be able to get to the following on your own:

class Orders:
    def __init__(self, reverse=False):
        self._orders = {}
        self._reverse = reverse

    def add(self, order):
        self._orders[order[0]] = order
        return True

    def remove(self, price):
        try:
            self._orders.pop(price)
            return True
        except KeyError:
            return False

class Ledger:
    def __init__(self):
        self.ask = Orders()
        self.bid = Orders(True)


However as __getitem__ can take both an int and a slice, you'll have to take that into account.
You don't need heapq and your usage of it was completely broken as you get a list back not an item.
You can do everything with sorted and itertools.islice.
And makes things simpler.

You want to:

  • Sort the orders' dictionary.



  • If is instance of int:



  • Perform an islice of one item.



  • Return the dict's item.



  • If the key isn't an instance of slice:



  • Raise TypeError.



  • Otherwise perform an islice of the slices start, stop and step.



  • Return the sliced orders.



Or in Python:

def __getitem__(self, key):
    keys = sorted(self._orders.keys(), reverse=self._reverse)
    if isinstance(key, int):
        key, = islice(keys, key, key + 1)
        return self._orders[key]
    elif not isinstance(key, slice):
        raise TypeError()
    return [
        self._orders[key]
        for key in islice(keys, key.start, key.stop, key.step)
    ]


Now that you've got a simpler base you can start improving performance.

If I were to want to improve the performance I'd change the dict that we use to a self made 'SortedDict'. Python already has an OrderedDict, and you can adapt the source-code for your needs.
This is as the only performance sink is the sorted in __getitem__.

But honestly I don't know if it's worth it.

Code Snippets

class Orders:
    def __init__(self, reverse=False):
        self._orders = {}
        self._reverse = reverse

    def add(self, order):
        self._orders[order[0]] = order
        return True

    def remove(self, price):
        try:
            self._orders.pop(price)
            return True
        except KeyError:
            return False

class Ledger:
    def __init__(self):
        self.ask = Orders()
        self.bid = Orders(True)
def __getitem__(self, key):
    keys = sorted(self._orders.keys(), reverse=self._reverse)
    if isinstance(key, int):
        key, = islice(keys, key, key + 1)
        return self._orders[key]
    elif not isinstance(key, slice):
        raise TypeError()
    return [
        self._orders[key]
        for key in islice(keys, key.start, key.stop, key.step)
    ]

Context

StackExchange Code Review Q#134427, answer score: 4

Revisions (0)

No revisions yet.