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:

enter image description here

and a description of the files:


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 []


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]))


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))

1 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.

  • \$\begingroup\$ 4. When you buy a share, you get a positive amount of share, but a negative amount of currency is going out of your account. So if I create a function "compute_position" using the trade type, this time the value of Buy and Sell make sense... \$\endgroup\$
    – Octoplus
    Oct 4, 2016 at 16:46
  • \$\begingroup\$ 2. Indeed, in C# too everything has to be a member of a class. I try to respect the Single Reponsibility Principle and I still have some difficulties figuring out what to put in a module. 5. Yes, I tried to add other fields not used yet, because I did not want to provide a too simplistic example. I will provide complete code on my next questions. \$\endgroup\$
    – Octoplus
    Oct 4, 2016 at 16:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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