Computing the profit or loss from a list of trades

Question

I am coming from C# and I am learning how to use Python more seriously.

So I decided to try a little side project to help me see how much I earn (or lose) from my trades on the stock market.

I would like some advice about how I structured my project, in C# I am used to put each class in its own file, but I read that in Python we work with module, so I put several classes in the trades.py module. But does it make sense to put the trade importer in the same module as the calculator?

Here is a screenshot of my project hierarchy so far:

and a description of the files:

trades.py

import calculator.trades as t


from enum import Enum
import numpy as np


class TradeType(Enum):
    Buy = 1
    Sell = -1


class Trade:
    def __init__(self, id, trade_date, type, quantity, price, currency_id, company_id, account_id):
        self.id = id
        self.type = type
        self.account_id = account_id
        self.company_id = company_id
        self.currency_id = currency_id
        self.trade_date = trade_date
        self.quantity = quantity
        self.price = price


class TradePnlCalculator:
    def compute_pnl(self, trades):
        signed_quantities = np.array([-1 * t.type.value * t.quantity for t in trades])
        prices = np.array([t.price for t in trades])
        return np.dot(signed_quantities, prices)


class TradeImporter:
    def load_trades(self):
        # TODO Read the trades from a file
        return []

test_tradePnlCalculator.py

from unittest import TestCase

from calculator.trades import Trade, TradeType, TradePnlCalculator
import datetime


class TestTradePnlCalculator(TestCase):

    def setUp(self):
        now = datetime.datetime.now()
        self.trade1 = Trade(1, trade_date=now, type=TradeType.Buy, quantity=100, 
                            price=34.5, currency_id=1, company_id=1, account_id=1)
        self.trade2 = Trade(2, trade_date=now, type=TradeType.Sell, quantity=100, 
                            price=37.4, currency_id=1, company_id=1, account_id=1)

        self.calculator = TradePnlCalculator()

    def test_compute_pnl_returns_zero_when_there_is_no_trades(self):
        self.assertEquals(0, self.calculator.compute_pnl([]))


    def test_compute_pnl_returns_negative_flow_for_single_buy_trade(self):
        self.assertEquals(-3450, self.calculator.compute_pnl([self.trade1]))

    def test_compute_pnl_returns_positive_flow_for_single_sell_trade(self):
        self.assertEquals(3740, self.calculator.compute_pnl([self.trade2]))

    def test_compute_pnl_returns_correct_value_for_cancelling_trades(self):
        self.assertEquals(3740-3450, self.calculator.compute_pnl([self.trade1, self.trade2]))

run.py

importer = t.TradeImporter()
trades = importer.load_trades()

calculator = t.TradePnlCalculator()
pnl = calculator.compute_pnl(trades)

print('The profit for the trades is {} euros'.format(pnl))

Answer 1

This is too sketchy to be able to do a proper review. But just based on what you've written in the post:

1. None of this code updates the attributes of a Trade. So you could use a collections.namedtuple which avoids some duplication:

   Trade = namedtuple('Trade', 'id trade_date type quantity price currency_id '
                      'company_id account_id')
   

2. In some languages (especially Java) everything has to be a member of a class. But this isn't the case in Python: you can just write a function if you want, without having to make a class whose only role is to act as a container for your function.

   So here there's no need for the TradePnlCalculator class — you could just define a function compute_pnl. And there's no need for the TradeImporter class — you could just define a function load_trades.

3. In compute_pnl, you're not gaining anything from using NumPy — it costs more to build the NumPy arrays than you gain in faster operations. It would be simpler and faster to write:

   -sum(t.type.value * t.quantity * t.price for t in trades)
   

4. The need for the negation of the result of this computation suggests that you have the values for Buy and Sell the wrong way round.

5. The code in compute_pnl doesn't consult the currency_id so it would go wrong if given a list of trades in a mixture of currencies.