6
\$\begingroup\$

I am learning to work with OOP design patterns and so I challenged myself with the idea of creating a way to generate a pdf invoice based on some information entered. So, this is what I have done so far and would like to have some review on my approach and design.

Invoice.py

import fpdf
from datetime import datetime

class Creator:
    def __init__(self,first_name,last_name,email,phone_num,address,city,country):
        self.first_name = first_name
        self.last_name = last_name
        self.email = email
        self.phone_num = phone_num
        self.address = address
        self.city = city
        self.country = country

class Organization:
    def __init__(self,name,address,city,country):
        self.name = name
        self.address = address
        self.city = city
        self.country = country

class BankAccountDetail:
    def __init__(self,account_name,account_num,currency,bank_name,branch,branch_addr):
        self.account_name = account_name
        self.account_num = account_num
        self.currency = currency
        self.bank_name =bank_name
        self.branch = branch
        self.branch_addr = branch_addr

class Project:

    def __init__(self,name,description,amount):
        self.name = name
        self.description = description
        self.amount = amount

class Invoice:
    '''
    Invoice class used to model a invoice object which is a composition of
    1. Creator Object
    2. Organization Object
    3. Project Object
    4. BankDetail Object
    '''

    def __init__(self,invoice_num,creator,organization,project,bankaccountdetail):
        self.invoice_num = invoice_num
        self.creator = creator
        self.organization = organization
        self.project = project
        self.bankaccountdetail = bankaccountdetail

class File:
    def __init__(self,filename,font_size,line_height,orientation):
        self.filename = filename
        self.font_size = font_size
        self.line_height = line_height
        self.orientation = orientation


class PdfInvoice(Invoice):
    '''
    Inherits from the Parent Invoice class and has an extra feature
    1. File Object : Used to specify some basic details about the file
    '''
    def __init__(self,invoice_num,creator,organization,project,bankaccountdetail,file):
        super().__init__(invoice_num,creator,organization,project,bankaccountdetail)
        self.file = file

    def generate_pdf(self):
        dt = datetime.now()
        date = dt.date()
        pdf = fpdf.FPDF(format=self.file.orientation)
        pdf.add_page()
        pdf.set_font("Arial", size=self.file.font_size)
        pdf.write(self.file.line_height,"Invoice Number #")
        pdf.write(self.file.line_height,self.invoice_num)
        pdf.ln()
        pdf.write(self.file.line_height,"Date Invoiced #")
        pdf.write(self.file.line_height,str(date))
        pdf.ln()
        pdf.write(self.file.line_height, "Billed By #")
        pdf.write(self.file.line_height,"{}{}".format(self.creator.first_name,self.creator.last_name))
        pdf.ln()
        pdf.write(self.file.line_height,"Address #")
        pdf.write(self.file.line_height,self.creator.address)
        pdf.ln()
        pdf.write(self.file.line_height, "City #")
        pdf.write(self.file.line_height, self.creator.city)
        pdf.ln()
        pdf.write(self.file.line_height,"Country #")
        pdf.write(self.file.line_height,self.creator.country)
        pdf.ln()
        pdf.write(self.file.line_height, "Email #")
        pdf.write(self.file.line_height, self.creator.email)
        pdf.ln()
        pdf.write(self.file.line_height, "Phone Number #")
        pdf.write(self.file.line_height, self.creator.phone_num)
        pdf.ln()
        pdf.write(self.file.line_height,"Billed To #")
        pdf.ln()
        pdf.write(self.file.line_height,"Organization Name #")
        pdf.write(self.file.line_height,self.organization.name)
        pdf.ln()
        pdf.write(self.file.line_height, "Organization Address #")
        pdf.write(self.file.line_height, self.organization.address)
        pdf.ln()
        pdf.write(self.file.line_height, "Organization City #")
        pdf.write(self.file.line_height, self.organization.city)
        pdf.ln()
        pdf.write(self.file.line_height, "Organization Country #")
        pdf.write(self.file.line_height, self.organization.country)
        pdf.ln()
        pdf.write(self.file.line_height, "Comments #")
        pdf.write(self.file.line_height, self.project.description)
        pdf.ln()
        pdf.write(self.file.line_height, "Amount #")
        pdf.write(self.file.line_height,str(self.project.amount))
        pdf.ln()
        pdf.write(self.file.line_height,'Account details ')
        pdf.ln()
        pdf.write('Account Name #')
        pdf.write(self.file.line_height,self.bankaccountdetail.account_name)
        pdf.ln()
        pdf.write('Account Number #')
        pdf.write(self.file.line_height,self.bankaccountdetail.account_num)
        pdf.ln()
        pdf.write('Account Currency #')
        pdf.write(self.file.line_height, self.bankaccountdetail.currency)
        pdf.ln()
        pdf.write('Bank Name #')
        pdf.write(self.file.line_height, self.bankaccountdetail.bank_name)
        pdf.ln()
        pdf.write('Branch Address #')
        pdf.write(self.file.line_height, self.bankaccountdetail.branch_addr)
        pdf.ln()
        pdf.output(self.file.filename)

creator = Creator('Test','User','test@gmail.com',
         '099006789','Joans Apartment, 123 Test road','Nairobi','Kenya')

organization = Organization('Test Org','Ndemi Road Kilimani', 'Nairobi','Kenya')

bank_detail = BankAccountDetail('Test User','999999678','KES',
                                  'Test Bank','Kenya','BRANCH  Way, ABC Place')

file = File("Invoice.pdf",12,5,"letter")

project = Project('Ecommerce site','Worked on the ecommerce site',10.900)

pdf_inv = PdfInvoice('1393939',creator,organization,project,bank_detail,file)
pdf_inv.generate_pdf()

UML diagram

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Is this supposed to work in Python 2 or Python 3? Please add the according tag to your question. \$\endgroup\$ – AlexV Jun 14 at 14:03
  • \$\begingroup\$ @AlexV:- Thank you. I have added the tag. \$\endgroup\$ – Anastacia Jun 14 at 14:18
  • 1
    \$\begingroup\$ Rendering text onto the PDF using one statement per string is quite tedious. Consider using a higher-level library or templating language such as ReportLab. \$\endgroup\$ – 200_success Jun 14 at 18:09
  • \$\begingroup\$ Welcome to Code Review! Please see What to do when someone answers. I have rolled back Rev 5 → 4 \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Jun 14 at 22:21
7
\$\begingroup\$

Data classes

Since you are using the classes as immutable data containers, it would be possible to significantly cut down the amount of code you have to write to create all of them using namedtuple from the collections module:

from collections import namedtuple

Creator = namedtuple("Creator", ["first_name", "last_name", "email", "phone_num",
                     "address", "city", "country"])


Organization = namedtuple("Organination", ["name", "address", "city", "country"])


BankAccountDetail = namedtuple("BankAccountDetail", ["account_name", "account_num",
                               "currency", "bank_name", "branch", "branch_addr"])


Project = namedtuple("Project", ["name", "description", "amount"])


File = namedtuple("File", ["filename", "font_size", "line_height", "orientation"])

dataclasses might also be used to get a similar result.


Code duplication

There is a massive amount of duplicate code in generate_pdf. You could layout the document using a list and string formatting and then iterate over that list to finally write it to a file. Let me give you a sketch of what I mean (Note: the code below is untested):

def generate_pdf(self):
    dt = datetime.now()
    date = dt.date()
    pdf = fpdf.FPDF(format=self.file.orientation)
    pdf.add_page()
    pdf.set_font("Arial", size=self.file.font_size)

    pdf_content = [
        f"Invoice Number #{self.invoice_num}", 
        f"Date Invoiced #{date}",
        # and so on and so forth
    ]

    for line in pdf_content:
        pdf.write(self.file.line_height, line)
        pdf.ln()

    pdf.output(self.file.filename)

The code uses f-string, which are available since Python 3.6. If you are using an older version of Python you will have to use .format instead such as you do in several places already.

There might be even better ways to do this, but I have no specific knowledge about that special library.


Misc

It might be a good idea to have a look at the official Style Guide for Python Code (often just called PEP8) for short. It's a collection of style recommendations and other paradigms that allow you to write compelling and visually appealing Python code.

file in file = File("Invoice.pdf", 12, 5, "letter") is not a good variable name since you overwrite Python's file command with this. At least append a _ to make it file_ or choose another name altogether.

Maybe it would also be worth to have a look at if __name__ == "__main__": to separate the "library part" from the example code.

from datetime import datetime
from collections import namedtuple

import fpdf


Creator = namedtuple("Creator", ["first_name", "last_name", "email", "phone_num",
                     "address", "city", "country"])

# all the other code ...

if __name__ == "__main__":
    creator = Creator('Test', 'User', 'test@gmail.com', '099006789',
                    'Joans Apartment, 123 Test road', 'Nairobi', 'Kenya')

    organization = Organization('Test Org', 'Ndemi Road Kilimani', 'Nairobi',
                                'Kenya')

    bank_detail = BankAccountDetail('Test User', '999999678', 'KES', 'Test Bank',
                                    'Kenya', 'BRANCH  Way, ABC Place')

    file = File("Invoice.pdf", 12, 5, "letter")

    project = Project('Ecommerce site', 'Worked on the ecommerce site', 10.900)

    pdf_inv = PdfInvoice('1393939', creator, organization, project, bank_detail,
                        file)
    pdf_inv.generate_pdf()

\$\endgroup\$
  • \$\begingroup\$ @AlexV- This is so helpful. I really appreciate it. \$\endgroup\$ – Anastacia Jun 14 at 20:53
1
\$\begingroup\$

Might want to look at LaTex. It's a lot less code and it IS possible to add comments in, so it's definetly possible to use it as both a template and a way to generate a pdf!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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