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','[email protected]',
'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()
-
\$\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– AlexV2019年06月14日 14:03:45 +00:00Commented Jun 14, 2019 at 14:03
-
\$\begingroup\$ @AlexV:- Thank you. I have added the tag. \$\endgroup\$Anastacia– Anastacia2019年06月14日 14:18:34 +00:00Commented Jun 14, 2019 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– 200_success2019年06月14日 18:09:28 +00:00Commented Jun 14, 2019 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ᴇᴌᴀ– Sᴀᴍ Onᴇᴌᴀ ♦2019年06月14日 22:21:42 +00:00Commented Jun 14, 2019 at 22:21
2 Answers 2
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', '[email protected]', '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()
-
\$\begingroup\$ @AlexV- This is so helpful. I really appreciate it. \$\endgroup\$Anastacia– Anastacia2019年06月14日 20:53:09 +00:00Commented Jun 14, 2019 at 20:53
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!
Explore related questions
See similar questions with these tags.