patternpythonMinor
Tracking orders efficiently using a Python class
Viewed 0 times
trackingefficientlyusingpythonclassorders
Problem
I've written a
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,
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.
And so honestly I'd entirely re-write this.
Without
However as
You don't need
You can do everything with
And makes things simpler.
You want to:
Or in Python:
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
But honestly I don't know if it's worth it.
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.
bidsandaskschange three things, and so you could merge these.
bidandaskhave the same interface. Make a class for it.
self._bid_ladderis the equivalent ofself._bin_orders.keys().
- Both
_addand_removecan be changed to be a couple of lines long by removing the 'ladders'.
bidandaskwould 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
isliceof one item.
- Return the dict's item.
- If the key isn't an instance of
slice:
- Raise
TypeError.
- Otherwise perform an
isliceof 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.