Simple OOP currency converter I am learning about OOP so it would be great if you can give me feedback on how to improve my code and OOP design.

This is a currency converter. Firstly, it will call a method to add 2 currencies and commission information and then it converts to another currency whose input is source currency and amount:

import pytest


class Bank:

    def addRate(self,first, second, rate):
        self.first_currency = first
        self.second_currency = second
        self.rate = rate

    def addCommission(self,commision):
        self.commission = commision


    def convert(self, currency, amount):
        if currency == self.first_currency:
            return (amount / self.rate) * (1-self.commission)
        else:
            return (amount * self.rate) * (1-self.commission)

@pytest.fixture()
def bank():
    return Bank()

def test_Bank_addRate(bank):

    bank.addRate("USD","GBP",2)
    assert bank.first_currency == "USD"
    assert bank.second_currency == "GBP"
    assert bank.rate == 2

def test_Bank_addCommision(bank):
    bank.addCommission(0.015)
    assert bank.commission == 0.015


def test_Bank_convert(bank):
    bank.addRate("USD", "GBP", 2)
    bank.addCommission(0.015)
    assert bank.convert("USD", 100) == 49.25
First: welcome to CodeReview! I hope that you get good feedback.

About your code. Your usage of Python is not terrible; most of your problems are conceptual, about the decisions you've made about your class. Looking at the method signatures from the outside, one would guess that addRate() can be called multiple times per single Bank instance, and that convert would then allow for any currency to be converted. That isn't the case; instead, your "Bank" class is actually closer to an "ExchangeRate" class.

There are two ways to go, here. Either make your class an ExchangeRate, in which case


you need to rename the class
convert, rather than accepting the name of a currency, should perhaps accept a forward boolean. Or, if you want to keep passing the name of a currency, it's important to make the argument name more clear, i.e. dest_currency.
Rename your addCommission to setCommission. "add" implies having more than one.
Rename your addRate to setRate. "add" implies having more than one.


Or, keep a "Bank" class, in which case:


you need to change the way that you store your exchange rates, probably to a dictionary-based system instead of a single float member
convert would accept both a source and destination currency
addRate's signature would stay the same, but its contents would change to use your dict.