Stock statement generator in Python

I applied to a startup recently, as a coding exercise they asked me to produce the output below with the given input, specifically making it easy to understand, extend, and without performing any unnecessary actions. I emailed this solution and got no response. The code works but I suspect there are problems with the design. Any and all criticisms are welcome, anything you could possibly object to, if it's awful by all means please let me know.

input:

actions = [{'date': '1992/07/14 11:12:30', 'action': 'BUY', 'price': '12.3', 'ticker': 'AAPL', 'shares': '500'},
{'date': '1992/09/13 11:15:20', 'action': 'SELL', 'price': '15.3', 'ticker': 'AAPL', 'shares': '100'},
{'date': '1992/10/14 15:14:20', 'action': 'BUY', 'price': '20', 'ticker': 'MSFT', 'shares': '300'},
{'date': '1992/10/17 16:14:30', 'action': 'SELL', 'price': '20.2', 'ticker': 'MSFT', 'shares': '200'},
{'date': '1992/10/19 15:14:20', 'action': 'BUY', 'price': '21', 'ticker': 'MSFT', 'shares': '500'},
{'date': '1992/10/23 16:14:30', 'action': 'SELL', 'price': '18.2', 'ticker': 'MSFT', 'shares': '600'},
{'date': '1992/10/25 10:15:20', 'action': 'SELL', 'price': '20.3', 'ticker': 'AAPL', 'shares': '300'},
{'date': '1992/10/25 16:12:10', 'action': 'BUY', 'price': '18.3', 'ticker': 'MSFT', 'shares': '500'}]

stock_actions = [{'date': '1992/08/14', 'dividend': '0.10', 'split': '', 'stock': 'AAPL'},
{'date': '1992/09/01', 'dividend': '', 'split': '3', 'stock': 'AAPL'},
{'date': '1992/10/15', 'dividend': '0.20', 'split': '', 'stock': 'MSFT'},
{'date': '1992/10/16', 'dividend': '0.20', 'split': '', 'stock': 'ABC'}]


output:

`On 1992-07-14, you have:
- 500 shares of AAPL at $12.30 per share
- $0 of dividend income
Transactions:
- You bought 500 shares of AAPL at a price of $12.30 per share
On 1992-08-14, you have:
- 500 shares of AAPL at $12.30 per share
- $50.00 of dividend income
Transacti

The code generally looks all right – you can definitively program. However, there are many details that show that you're fairly new to Python. Some potential problems are fairly minor, but could indicate that you are not an experienced software developer.

The most pressing problem is that you are using strings and dictionaries for everything. For many of these cases you should use float or int or a custom object instead. Because you only use strings you convert your values repeatedly, all over the place, which clutters your code. Anything else in your code is fairly forgiveable and can be corrected quickly, but this here really sticks out.

The fix here is to separate input from your domain model from your output. The domain model represents the things that are actually interesting for your program, like stocks and trading actions. These should be represented in a way that are convenient for the problems you are solving. Usually, this means creating a class for each concept you are modelling.

When data is loaded into your program, you convert the data to your domain model at the application boundary. When you output data, you convert the domain model to the required data format – again, only at the application boundary, not in the middle of things.

Your code lacks this architecture. One could even say that your code lacks any architecture. You have a single class that does everything. Except for the instance fields stocks and dividend_income, your methods would work just as well as free functions, no class needed.

Classes should inherit from object. Instead of class Portfolio, I'd expect to see class Portfolio(object).

Minor: All your docstrings have leading spaces. This is unnecessary.

Minor: All your function docstrings contain a type signature. According to the PEP-257 docstring conventions, the first line should contain a short summary of the description of the behaviour:


The one-line docstring should NOT be a "signature" reiterating the function/method parameters (which can be obtained by introspection). Don't do:

def function(a, b):
    """function(a, b) -> list"""

This type of docstring is only appropriate for C functions (such as built-ins), where introspection is not possible. However, the nature of the return value cannot be determined by introspection, so it should be mentioned. The preferred form for such a docstring would be something like:

def function(a, b):
    """Do X and return a list."""

(Of course "Do X" should be replaced by a useful description!)

In any case, documenting the type of the self parameter to a method is misleading. Since no such parameter is explicitly passed when calling a method, it's best to ignore self in the docs.

Don't use == to compare with None. Instead of equality, test for identity with is, i.e. if x is None: ....

You are using dictionaries as ad-hoc objects. This is sometimes OK, but generally makes the code more difficult to maintain. It is easy to accidentally introduce a bug by mistyping one of the keys. Instead, define a simple class. Convert the input data to objects of this class first. This also avoids having to convert the strings to ints or floats for each calculation, thus reducing clutter.

This line is unacceptable:

stock_to_buy['value'] = "{0:.2f}".format((((float(stock_to_buy['value']) * int(stock_to_buy['shares'])) + (float(price) * int(shares))) / (int(stock_to_buy['shares']) + int(shares))))

For starters, it's too long. This is not just problematic because long lines are more difficult to read, but because this line does too much. I've already mentioned that the conversions should be done once when loading the input data. Once we do that, this is simplified to

stock_to_buy['value'] = "{0:.2f}".format((((stock_to_buy['value'] * stock_to_buy['shares']) + (price * shares)) / (stock_to_buy['shares'] + shares)))

But the high number of parentheses (((( is still quite difficult to read. Some of those parentheses are entirely unnecessary. If they don't contribute to clarity, they should be omitted. But more importantly, you should extract part of the calculation into separate variables with self-documenting names. Each line should do just one thing. How these variables should be named depends on the problem domain; since I'm not familiar with shares I'll not suggest anything here.

You are using + to concatenate strings. Instead, use the .format() method to insert data into a template. This

"You bought " + shares + " shares of " + stock + " at a price of $" + "{0:.2f}".format(float(price)) + " per share"

should be

"You bought {shares} shares of {stock} at a price of ${price:.2f} per share".format(
    shares=shares,
    stock=stock,
    price=float(price))

Note that with many arguments to format(), using named placeholders makes the code more easy to read.

If you need comments to explain what a variable means, you should try to find a

StackExchange Code Review Q#155703, answer score: 15